-
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
Bazel Examples configuration is failing with Bazel@HEAD #18771
Comments
/cc @aoeui @justinhorvitz |
Writing from somewhere in Atlantic ocean over spotty satellite internet so can't fix this right away. Most likely a known consequence of a recent change. Non-convergent transitions -- a transition that when applied repeatedly -- repeatedly generates novel configurations leads to an infinitely expanding graph. Ideally, there would be a clear error message for this condition. Preferred solution is to make the transition idempotent. Anything that gets cleaned up in this way can't regress because anything exhibiting this behavior leads to OOMs. |
Let me know if you want to roll back 52dbdc7 until Shahan is back and can give this some attention including more helpful failure messages and clean up violations. I don't know where to find the configuration transition that is leading to infinite config expansion (presumably adding a repeated flag) but if there's just one place perhaps that can be changed. |
I think Bazel example is the only broken downstream project, so it's not that urgent. The configurations is located under https://github.com/bazelbuild/examples/tree/main/configurations |
https://github.com/bazelbuild/examples/blob/cc8af817b441a5aca9eb215cee51ca12cb141e3a/configurations/read_attr_in_transition/defs.bzl#L8 is definitely a culprit (not sure if there are others). I believe Shahan is planning to fail fast on a non-convergent transition like that, and if so the documentation and examples should of course be updated. Until then you could try changing that example to not append if the string already ends with |
/cc @gregestren |
I'm not sure I can completely follow this discussion, but is it correct that all transitions are expected to be idempotent? Assuming a non-idempotent transition is only applied at the particular edge it is attached to, how would it end up creating an infinite number of configurations? If non-idempotent transitions are expected to fail in one way or another, then this would break rulesets such as rules_fuzzing (https://github.com/bazelbuild/rules_fuzzing/blob/9865504b549e86ccfb4713afcc1914c982567f05/fuzzing/private/binary.bzl#L58, where |
I believe the requirement would only be limited to "self transitions" like the example I linked to. |
@fmeum |
@gregestren @justinhorvitz Thanks, I missed that part. It's still slightly surprising to me as I understood "self transitions" to essentially be "incoming edge transitions", but all examples I have seen have been outgoing edge transitions and thus aren't affected (including rules_fuzzing). |
Sorry, I was wrong. There are some other projects in downstream with hanging or OOM errors, so I believe it's better to rollback the change first. |
Let's roll back and then consider whether we should do a more thorough cleanup of external projects (this approach seemed to work within google) or pivot to an alternative approach (there is another I think will work). |
*** Reason for rollback *** #18771 PiperOrigin-RevId: 544025702 Change-Id: I5c036cda4536f86088f259391cdb7c58ef04df6d
I have a 64 cores, 128G memory physical server. Want to known use multi cores to build bazel with bazel-dist-version.zip source packages |
*** Reason for rollback *** Roll forward with fix. The original CL caused infinite expansion of rule transitions when they were non-convergent. The fix adds idempotency checking to transitions when they are non-noop and flags them to not apply a rule transition via the new ConfiguredTargetKey.shouldApplyRuleTransition property. This is discussed more thoroughly in the code comments of ConfiguredTargetKey and IdempotencyChecker. *** Original change description *** Automated rollback of commit 52dbdc7. *** Reason for rollback *** #18771 PiperOrigin-RevId: 545664983 Change-Id: Id1e46b61506edb861e76ff5f3858e3c95aaaa407
*** Reason for rollback *** #18771 PiperOrigin-RevId: 544025702 Change-Id: I5c036cda4536f86088f259391cdb7c58ef04df6d
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/3123#0188f574-6d64-41f8-9bc1-f1128df8d993
Platform : Macos, Lts, Windows
Logs :
Steps :
Bisect result:
Culprit : 52dbdc7
CC Greenteam @comius
The text was updated successfully, but these errors were encountered: