-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
//src/test/shell/bazel/android:android_ndk_integration_test doesn't work with remote caching #4663
Comments
What is the error here? Now that we're using BuildKite, we should re-enable this as soon as possible. |
Ping, can we disable remote caching just for this test? |
The error is We unfortunately can't disable remote caching just for one test, but we can completely disable remote caching until it's fixed? @philwo ? |
perhaps this is a problem with mac case sensitivity? the runfiles of this test should contain: |
@aj-michael this looks like it's running on ubuntu |
@buchgr can you please provide some steps on how we can reproduce this with a remote caching setup? Is there an existing remote cache server that we can use to reproduce this? |
Managed to repro:
|
I renamed the directory from
Deleted the
|
@buchgr ping, is this a problem with external repositories? The NDK external repository is a directory of symlinks to a local path. This test has been disabled for 3 weeks now and I'd like to help debug to get it back up again. |
Perhaps this is due to the fact that the test has an input that is a filegroup that contains a directory: |
@aj-michael Yes, I remember that directory inputs are very very problematic. They cannot be cached, the sandbox may fall over its own feet, Bazel has no clue when stuff inside that directory changes... Is there any way we could get rid of that directory filegroup? Maybe replace it with a glob? |
Thanks @aj-michael @philwo, let me try switching it to a glob. |
@philwo - my understanding is that directory inputs are something that is (unfortunately) supported in Bazel for historical reasons. Does remote caching not work with it at all? If so, I suspect there will be other users who are broken. Also, I wonder why our SDK integration tests do not have this problem. They have a glob with exclude_directories = 0: There are two reasons that we do not use a glob here:
Perhaps we can find a solution for 1, but I can't think of a solution for 2 |
For example, the NDK contains many paths with the string |
There's the However, I tried marking the integration test with this tag and it's still failing with the same error. Maybe it's not doing what I think it's doing? |
I have a partial change to properly support directory inputs, but haven't had time to finish it. Unfortunately, due to a change by @ola-rozenfeld, the |
It's change https://bazel-review.googlesource.com/c/bazel/+/29872 - it's incomplete because it's not returning the correct metadata and it's not adding the metadata to the MetadataHandler. That said, even with that change, I think it'll still require a patch to RemoteSpawnCache / RemoteSpawnRunner. |
|
There's a workaround that will let us enable it.Instead of using |
@buchgr I think your change accidentally disabled sandboxing on all platforms, as the RemoteSpawnRunner uses the normal LocalSpawnRunner as its fallback. |
…ld/bazel#4663" This reverts commit 2db59bb.
This seems to break in the downstream pipeline, e.g. |
The TreeNodeRepository served us well, but has become a bit dated over time and it's increasingly hard to understand and to fix bugs. Additionally, it was written with the initial intend of incrementally updating the merkle tree between actions, however internal performance analysis has shown that doing so is 1) hard to implement reliably and 2) would increase memory consumption multifold. This is a rewrite of the code that also fixes bugs like bazelbuild#4663. The merkle tree construction is split into two: - InputTree is an intermediate tree representation of a (sorted) list of inputs. - MerkleTree implements a visitor pattern over an InputTree and is responsible for serializing the InputTree to the protobuf merkle tree that's used by remote caching / execution. Benchmarks on my local machine have shown this implementation to consumes between 2-10x less CPU time than the current implementation.
The TreeNodeRepository served us well, but has become a bit dated over time and it's increasingly hard to understand and to fix bugs. Additionally, it was written with the initial intend of incrementally updating the merkle tree between actions, however internal performance analysis has shown that doing so is 1) hard to implement reliably and 2) would increase memory consumption multifold. This is a rewrite of the code that also fixes bugs like bazelbuild#4663. The merkle tree construction is split into two: - InputTree is an intermediate tree representation of a (sorted) list of inputs. - MerkleTree implements a visitor pattern over an InputTree and is responsible for serializing the InputTree to the protobuf merkle tree that's used by remote caching / execution. Benchmarks on my local machine have shown this implementation to consumes between 2-10x less CPU time than the current implementation. Tests by users have shown equally encouraging speedups [1]. [1] https://groups.google.com/d/msg/bazel-discuss/wPHrqm2z8lU/mzoRF236GQAJ
The TreeNodeRepository served us well, but has become a bit dated over time and it's increasingly hard to understand and to fix bugs. Additionally, it was written with the initial intend of incrementally updating the merkle tree between actions, however internal performance analysis has shown that doing so is 1) hard to implement reliably and 2) would increase memory consumption multifold. This is a rewrite of the code that also fixes bugs like bazelbuild#4663. The merkle tree construction is split into two: - InputTree is an intermediate tree representation of a (sorted) list of inputs. - MerkleTree implements a visitor pattern over an InputTree and is responsible for serializing the InputTree to the protobuf merkle tree that's used by remote caching / execution. Benchmarks on my local machine have shown this implementation to consumes between 2-10x less CPU time than the current implementation. Tests by users have shown equally encouraging speedups [1]. [1] https://groups.google.com/d/msg/bazel-discuss/wPHrqm2z8lU/mzoRF236GQAJ
The TreeNodeRepository served us well, but has become a bit dated over time and it's increasingly hard to understand and to fix bugs. Additionally, it was written with the initial intend of incrementally updating the merkle tree between actions, however internal performance analysis has shown that doing so is 1) hard to implement reliably and 2) would increase memory consumption multifold. This is a rewrite of the code that also fixes bugs like #4663. MerkleTree implements a visitor pattern over an InputTree and is responsible for serializing the InputTree to the protobuf merkle tree that's used by remote caching / execution. Benchmarks on my local machine have shown this implementation to consumes between 2-10x less wall time than the current implementation. Tests by users have shown equally encouraging speedups [1]. [1] https://groups.google.com/d/msg/bazel-discuss/wPHrqm2z8lU/mzoRF236GQAJ Closes #7583. PiperOrigin-RevId: 237421145
…ld/bazel#4663" This reverts commit 2db59bb.
Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2.5 years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team ( |
This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team ( |
|
The text was updated successfully, but these errors were encountered: