Scalable Barriers (Again) - #467
Conversation
…ith complete TLA specifications comprehensively verified with model checking through a small number of nodes and transitions
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## mbauer-fix-multicast #467 +/- ##
========================================================
- Coverage 34.94% 33.81% -1.14%
========================================================
Files 202 202
Lines 44162 45648 +1486
Branches 15266 16012 +746
========================================================
+ Hits 15433 15434 +1
- Misses 27383 29031 +1648
+ Partials 1346 1183 -163 ☔ View full report in Codecov by Harness. |
I think the documentation is wrong and the behavior as previously written is intentional. Every barrier generation lives as an independent entity and arrival count adjustments don't flow forward. I don't know that we care: SCR is dead and we don't maintain any active manual CR codes. I just mention it because there may well be applications that rely on the implemented behavior and I personally never found it unintuitive. |
Again, maybe we don't care, but SCR did rely on this and perhaps manual CR codes would too. |
|
I'd be curious what the strategy is for keeping the TLA in sync with the C++ (or for that matter, verifying that the ever matched). As far as I know it's a manual process and the formal verification is only as good as your ability to line up the two implementations. |
|
What is the difference between this PR and Artem's old scalable barrier implementation? |
Note, this pull request does not change the documented semantics. The prior implementation was not faithfully implementing the described semantics. All we're doing here is ensure that the runtime matches the documentation, which in my opinion, is the better semantics for how
If there was anyone (and I doubt it), they should have reported it as a bug for not matching the documentation. ;) |
If you know of one you let me know. ;) The only one I've got is: ask the LLM to check that the implementation matches the TLA spec. In this case all the code has actually been synthesized from the spec itself (not the other way around) so it's guaranteed to match. We can say something like: always change the spec and then ask an LLM to generate code from it, but there's no way to enforce that. |
I would say that the main way it is different is that it is less buggy. ;) Artem's previous version is still disabled because no one ever went back and fixed all the bugs we were observing with it. This version is getting robustly tested at scale and is passing complex tests with lots of state transitions that exercise all the different states in the TLA+ model, so I have a lot more confidence that it is correct and we'll actually be able to deploy it in practice. Looking at the complexity of the TLA+ models (for both arrivals and notifications) I think it is safe to say that Artem's prior implementation didn't handle all the necessary cases. |
I want to be careful with this claim. This is not a "correctness by construction" argument because the LLM is not a deterministic function. (Neither is a human, for that matter.) There is no guarantee, even if the LLM really does start with the TLA+ code and then derive the C++ from that, that the code is ever going to match. We have at most an empirical argument that this is the case (if we run the code and see no hangs), but the assurance only goes one direction: if we do see a hang, we know the C++ code doesn't match (or the TLA+ doesn't describe what we think it describes), but if we don't see a hang, we'll never know if they really match 100% or if we're just waiting to discover some corner case. I still think it's better than before, and TLA+ is better than no TLA+, but I'd be more comfortable if we had a human actually read the new code, because who knows what's hiding in there. |
That may be true in principle, but I gave the LLM extremely strict instructions to ensure a 1-to-1 translation and then had it launch multiple validation agents to confirm the invariant, followed by me asking an entirely different LLM to also perform an audit to confirm the invariant. It's as close to true as it is going to be.
I don't think that's reasonable. There's about 5K lines of code changes and the complexity of the state machines are monstrous. Checking something like 7 nodes with 5 barrier generations and 3 different arrival patterns across those generations requires checking more than 100+M states and required a 24 hour TLC run to verify soundness. You're never going to be able to check it just by looking at the C++ code. I think the only way you can verify this is by running the code. I've done aggressive testing up to 128 nodes (512 ranks with 8 processes/node) on Eos and so far no bugs. Ask your favorite LLM to concoct the most aggressive test that you want and I'll even run it for you. No way is anyone going to find any bugs staring at this C++ code though. |
…ocal arrivals and all child arrivals have occurred



Rewrites Realm's barrier internals around two tree-structured protocols designed to scale to thousands of nodes, replacing the all-ranks-message-the-owner scheme. Both protocols were designed and formally verified in TLA+ before implementation; the specs, model-checking scenarios, and design documents ship in tla/barrier/ as the normative record.
Design
Verification
Every protocol rule (10 arrival + 8 notification) is backed by a mutation battery: disable the rule, and a model-checking scenario chosen for exactly that failure catches it (tla/barrier/battery.py, 26 rows, ~20 min). The models found real bugs before the code did — including a double-count safety bug where re-parenting a node mid-generation counted its arrivals twice, and several liveness strands in plan-switch races — and drove the fixes (pinned report edges, dead-plan install guards, retroactive outsider signaling). They also surfaced one pre-existing bug: alter_arrival_count is documented as persistent across generations but was applied to a single generation; this PR fixes it to match the documented contract (note: behavioral change, and the API previously had zero test coverage anywhere).
Implementation notes
Testing