Repository navigation
Keep a sample transfer bounded when inputs notify from other threads - #14
Merged
Merged
Conversation
Sample.java's monitor blocks every other notifier while a transfer reads an input, so the do/while only re-reads for notifications the read itself raised, and a surplus notification is discarded by the thread that raised it. Ebb reads outside the lock, so a notification from another thread landing mid-read made the transfer read again, and a surplus was left for whoever held the slot to pay. An input notified as fast as it can be read then kept one transfer going indefinitely. That is what timed out sample-never-derefs-one-input-twice-at-once on CI with jolt 0.8.21 and later. Thread/sleep there takes ~1.25ms for (Thread/sleep 0 n) where it used to return at once, which put the test's notifier and its reads at the same rate, and the transfer ended at a random point: up to 2.5s locally under load, past the 15s deadline on a 2-core runner. A read now marks itself in a carried context-local, so notifications from inside it (including an ap or cp input's owner fiber serving the deref) are still the read's own. Anyone else's notification during a transfer is recorded as late and becomes the mark for the next transfer, and an outside surplus waits for the slot and discards for itself.
With an outside notifier draining its own surplus, the slot was freed between its discards and the notifier took it straight back before the transfer waiting on the gate had woken up. On a 2-core runner the transfer never got in and the new surplus test timed out every run. A transfer that loses the slot now asks for it, and the holder hands it over on release instead of freeing it.
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.
sample-never-derefs-one-input-twice-at-once times out intermittently on CI with jolt 0.8.21 and nightly (#13). Dispatch runs of ci-nightly-jolt: 0.8.2, 0.8.10, 0.8.16 and 0.8.20 passed every run (9/9 on 0.8.20), 0.8.21 failed 2 of 8, nightly failed 3 of 8.
Root cause is in sample. Sample.java's monitor blocks every other notifier while a transfer reads an input, so its do/while only re-reads for notifications the read itself raised, and a surplus notification is discarded by the thread that raised it. Ebb reads outside the lock, so a notification from another thread that lands mid-read made the transfer read again, and a surplus was left for whoever held the slot. An input notified about as fast as it can be read keeps one transfer reading for as long as that lasts.
The test's input sits right on that line since jolt 0.8.21. jolt 45bfe3df rounds deadlines up, so
(Thread/sleep 0 n)went from returning at once to ~1.25ms (the nanos are ignored), which made the notifier's period and the read's length the same. The transfer then ends at a random point: the four-sample probe took up to 2.5s locally (8.1s under load) on nightly versus 65ms on 0.8.20, and on a 2-core runner it passes the 15s deadline. With the jolt sleep fixed to honour nanos (separate jolt PR) the notifier is 5x faster than the read and main's sample hangs every time.Fix:
Tests: two new stress tests. One has an in-protocol input re-notified from another thread during every read; before the fix it does 440k reads for 12 samples and times out. The other has a notifier that always outruns the reads; it timed out before too. Both pass now. The existing test is unchanged. Under load locally the probe now stays under 100ms per run. bin/test passes on nightly, 0.8.21 and 0.8.2.