Repository navigation
Conversation
plotnick
commented
Oct 21, 2026
Comment on lines
+57
to
+63
| /// Suppose we didn't store the successor, and this sled received | ||
| /// job ID `J` with `command = reboot`. The sled could easily die | ||
| /// before gossiping the full job; it would have written `J` to its | ||
| /// boundary record, but would not have any way to compute the successor | ||
| /// ID `K` unless *re-presented* with the full job for `J`. Thus, | ||
| /// the session would be wedged unless the operator took the (unusual, | ||
| /// undesirable) step of re-submitting a job just to advance the chain. |
Collaborator
Author
There was a problem hiding this comment.
@plaidfinch this is my best interpretation of the argument for storing the boundary job's successor's ID. I think it's compelling, but am always happy to remove unnecessary machinery if it's still not right. I do think being able to issue reboot as a command and then continue the session is an important use case, but if you think we don't need this mechanism to accomplish that, I would be very interested to hear why.
Collaborator
Author
There was a problem hiding this comment.
(I regret, as I usually do, my choice of the word "easily" there. "It's easy to imagine that ..." would be more accurate. I don't really have evidence either way, I just figured that anything network related could get cut off quickly and abruptly if the sled is going down.)
plotnick
force-pushed
the
locker-cleanups
branch
from
October 21, 2026 20:14
83078f5 to
8882f98
Compare
The problem with flock (used in the previous implementation of slot locks) is that a child forked to run a job inherits the locks via open fds until it execs. If the locker drops in that window, the slots stay locked and the next `Locker::new` over them fails with `Locked`, even though nothing in the process holds them any more. Reproduced by running the server's test binary with `--test-threads=16` in four concurrent instances with moderate CPU load. The locker-backed tests failed consistently with `EWOULDBLOCK` on fresh tempdir lock files; the same tests pass with these new fcntl locks. An fcntl lock belongs to the whole process, so the kernel cannot refuse a second locker constructed in the same process; we assume one locker per process rather than enforcing it. Co-Authored-By: Claude Mythos 5 <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com>
Fixes #93. Also reorganizes the source a bit to put the tenants (`boundary` and `bookmark`) in a `locker` module. Co-Authored-By: Claude Mythos 5 <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com>
An unreadable boundary record used to wedge the sled: the store marked itself untrusted, the state machine refused every local job, and recovery meant a clean slate. The wedge is unnecessary, because a sled without a record serves only sessions started after its own birth announcement, and no previous life can have run those. Starting over is safe, and the next commit overwrites the unreadable record. This removes the untrusted machinery, and with it the Commitment enum, which had collapsed into an Option. Co-Authored-By: Claude Mythos 5 <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com>
JobOutcome was an Option in disguise: Committed meant "no ending recorded" and Ended wrapped the status; the field is now status: Option<JobStatus>. Boundary::job was also an Option only for the v0 design, which wrote a jobless record at universe entry to persist the burned set; v2 writes the record at commit alone, so the job can never be absent, and Committed's fields fold directly into Boundary. The v0 record chain goes too: a v0 record only ever sat inside a version 0 envelope, which no longer decodes, so the conversion was unreachable. The v1 pin is regenerated, which is legitimate only while v1 exists on this unmerged branch alone. Co-Authored-By: Claude Mythos 5 <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com>
Fixes #87. Co-Authored-By: Claude Mythos 5 <noreply@anthropic.com> Co-Authored-By: Claude <noreply@anthropic.com>
plotnick
force-pushed
the
locker-cleanups
branch
from
October 22, 2026 20:53
8882f98 to
969e5d5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Locker and boundary work on top of #98, one commit each:
fcntladvisory record locks, which forked (job) children do not inherit. Fixes a known locker test flake and a potential problem in production under high load.Pre-release records on the racklettes won't decode under the versioned envelope, so should be discarded. Future migrations will be easier thanks to versioning.