Input prefetcher handles partial tree artifacts and nested artifacts incorrectly #16333
Labels
P2
We'll consider working on this in future. (Assignee optional)
team-Remote-Exec
Issues and PRs for the Execution (Remote) team
type: bug
Comments
coeuvre
added
type: bug
P2
|
In addition, it's possible for an action to consume only one of the files in a tree artifact (see |
tjgq
changed the title
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 13, 2022
…partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix bazelbuild#16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes bazelbuild#16987.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 13, 2022
…partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix bazelbuild#16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes bazelbuild#16987.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 13, 2022
…partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix bazelbuild#16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes bazelbuild#16987. PiperOrigin-RevId: 495050207 Change-Id: I7d29713085d2cf84ce5302394fc18ff2a96ec4be
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 13, 2022
…support partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix bazelbuild#16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes bazelbuild#16987. PiperOrigin-RevId: 495050207 Change-Id: I7d29713085d2cf84ce5302394fc18ff2a96ec4be
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Dec 13, 2022
…support partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix bazelbuild#16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes bazelbuild#16987. PiperOrigin-RevId: 495050207 Change-Id: I7d29713085d2cf84ce5302394fc18ff2a96ec4be
Wyverald
pushed a commit
that referenced
this issue
Dec 13, 2022
…support partial tree artifacts. (#17013) The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix #16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes #16987. PiperOrigin-RevId: 495050207 Change-Id: I7d29713085d2cf84ce5302394fc18ff2a96ec4be
hvadehra
pushed a commit
that referenced
this issue
Feb 14, 2023
…partial tree artifacts. The actions generated by SpawnActionTemplate and CppCompileActionTemplate contain individual TreeFileArtifacts, but not the respective TreeArtifact, in their inputs. This causes the input prefetcher to crash when building without the bytes, as it is only able to fetch entire tree artifacts and expects their input metadata to be available. This PR introduces an ActionInputPrefetcher#supportsPartialTreeArtifactInputs method signaling whether prefetching partial tree artifacts is supported; if not, the entire tree artifact is included in the inputs of any such actions. This is a temporary workaround to unblock the 6.0.0 release. The long-term plan is to fix #16333 to enable prefetching partial tree artifacts, and remove this workaround. Fixes #16987. PiperOrigin-RevId: 495050207 Change-Id: I7d29713085d2cf84ce5302394fc18ff2a96ec4be
|
Reopening because 25ba76c was just a workaround, and we have yet to fix it properly. |
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Feb 27, 2023
Due to the existence of partial trees (tree artifacts whose files are not all produced by the same action) and nested artifacts (artifacts whose output path is a strict prefix of another artifact's), concurrent calls to the prefetcher might write to the same directory in the output tree, so they must synchronize when making the output directory temporarily writable. Fixes bazelbuild#16333.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Feb 27, 2023
Due to the existence of partial trees (tree artifacts whose files are not all produced by the same action) and nested artifacts (artifacts whose output path is a strict prefix of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Feb 27, 2023
Due to the existence of partial trees (tree artifacts whose files are not all produced by the same action) and nested artifacts (artifacts whose output path is a strict prefix of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Feb 27, 2023
Due to the existence of partial trees (tree artifacts whose files are not all produced by the same action) and nested artifacts (artifacts whose output path is a strict prefix of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333.
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Feb 27, 2023
Due to the existence of partial trees (tree artifacts whose files are not all produced by the same action) and nested artifacts (artifacts whose output path is a strict prefix of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333.
|
@tjgq Should this be cherry-picked? |
|
Yes, but I'm not sure 6.1.0 is still accepting cherry-picks. If we decide to do an rc3, I'll cherry-pick it there. Otherwise, it will have to go into 6.2.0. |
ShreeM01
pushed a commit
to ShreeM01/bazel
that referenced
this issue
Mar 9, 2023
Due to the existence of templated tree artifacts (tree artifacts where each file is produced by a separate action) and nested artifacts (artifacts whose output path is a descendant of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333. PiperOrigin-RevId: 513205572 Change-Id: I827c4643643f63c9425e63bdf9177805c5f0f409
tjgq
added a commit
to tjgq/bazel
that referenced
this issue
Mar 10, 2023
Due to the existence of templated tree artifacts (tree artifacts where each file is produced by a separate action) and nested artifacts (artifacts whose output path is a descendant of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333. PiperOrigin-RevId: 513205572 Change-Id: I827c4643643f63c9425e63bdf9177805c5f0f409
ShreeM01
pushed a commit
that referenced
this issue
Mar 11, 2023
…17735) Due to the existence of templated tree artifacts (tree artifacts where each file is produced by a separate action) and nested artifacts (artifacts whose output path is a descendant of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes #16333. PiperOrigin-RevId: 513205572 Change-Id: I827c4643643f63c9425e63bdf9177805c5f0f409
|
@bazel-io fork 6.2.0 |
fweikert
pushed a commit
to fweikert/bazel
that referenced
this issue
May 25, 2023
Due to the existence of templated tree artifacts (tree artifacts where each file is produced by a separate action) and nested artifacts (artifacts whose output path is a descendant of another artifact's), the prefetcher must not assume every tree artifact is fetched as a whole. In addition, concurrent actions calling the prefetcher might write to the same directory in the output tree, so they must synchronize when making the directory temporarily writable. Fixes bazelbuild#16333. PiperOrigin-RevId: 513205572 Change-Id: I827c4643643f63c9425e63bdf9177805c5f0f409
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It's possible for artifacts to nest (i.e., to have a declare_file or declare_directory that descends from another declare_directory). If two nested inputs are presented to the input prefetcher, the common files will be downloaded twice and race against each other, potentially corrupting the input tree.
https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java;l=289;drc=05b97393ff7604f237d9d31bd63714f27fbe83f2
https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java;l=230;drc=05b97393ff7604f237d9d31bd63714f27fbe83f2
The text was updated successfully, but these errors were encountered: