Repository navigation
Let a waiting committer lead the group commit - #72
Open
venkat1701 wants to merge 3 commits into
Open
venkat1701 wants to merge 3 commits into
venkat1701 wants to merge 3 commits into
Conversation
Every commit used to hand its batch to a dedicated group-commit thread and wait to be woken again, even under async durability where nothing is synced. That was two thread wake-ups per commit. Now the first committer waiting on an unpublished commit leads: it syncs once for everything queued and publishes it in order, while the others wait. A lone committer finishes on its own thread, and concurrent sync commits still share one fsync.
A lone commit is published on its own thread, 1,600 concurrent commits come out in submission order sharing barriers, and a failed barrier fails every commit after it. The first test fails on the old committer.
…ns.md The group-commit section still said the feed is appended before the generation becomes current, which #49 reversed.
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.
Part of #46
What was slow: most of a small commit's time wasn't work, it was waiting. Every commit handed its batch to the dedicated
hstore-group-committhread and parked until that thread woke it again. That's two thread wake-ups per commit, even under async durability where nothing is synced. JFR on 8,100 single-update commits shows the committing thread parked about once per commit, for 1.1 ms on average, which was roughly 80% of the commit time on this loaded machine.What changed (leader commits, as agreed):
drain()andclose()lead too, so nothing stays queued.One trade-off: the fsync now runs on the committing thread, which in the server is usually a virtual thread. The JDK copes with a virtual thread blocked in file I/O by adding a carrier temporarily. That's in the docs.
Results, HStore only, scale 1, async, 3 alternating runs each against
main(2094e92), all checksums identical:ingest.nodeswrite.updateCaveat about
mixed.writep99: in 2 of 3 runs after the change it went from about 240 ms to 3.7 to 4.4 s. I traced it with JFR, and it isn't this change. The writer waited 4.4 s for the commit lock, which background compaction held duringTransactionManager.rewrite(#50). Every run, before and after, had exactly one background compaction pass. In the slow runs it fell inside the 10 s mixed window, which is why their mixed window wrote 117 MiB instead of 88. When it fell elsewhere, the new code did better than the old (p99 95 and 146 ms against 233 to 240 ms). Faster commits shift when the bytes-written trigger fires, so it lands in the window more often. The fix is #50, which I'm doing next.Tested:
./mvnw installpasses (95 tests); each commit compiles on its ownCrashRecoveryTest(16 crash points × WAL modes) passed 5 runs in a rowGroupCommitterTest:clinical-claims.hqlthrough sync commitshstore checkpasses; all 24 patients presentAlso fixes the group-commit section of
transactions.md, which still said the feed is appended before the generation becomes current; #49 reversed that.