Skip to content

Keep the parts of the old compaction work that still help - #82

Open
venkat1701 wants to merge 3 commits into
mainfrom
perf/kept-from-compaction-stack
Open

venkat1701 wants to merge 3 commits into
mainfrom
perf/kept-from-compaction-stack

Conversation

@venkat1701

Copy link
Copy Markdown
Collaborator

#73 to #76 sped up the old compaction, which walked the trees and rewrote the paths to moved pages. #81 replaced that with moving pages through the page directory and counting references, so most of that stack no longer applies and it is being closed. This PR keeps the three pieces that still help on the current code.

What changes

  • Inline posting lists are decoded without streams (from Walk trees for compaction without allocating per page #75). Each inline list used to go through a stream, a list that allows nulls and a second copy. It is now built once with List.of.
  • Nested trees are visited without rebuilding records (from Walk trees for compaction without allocating per page #75, adapted). mapRefs built a new record for every leaf value even when every reference mapped to itself. It now returns the same record in that case, and a new forEachRef visits the references without building anything. The reference counter's walk (TreeWalker.references) and the check that sets a leaf's REFERS_TO_PAGES flag now use forEachRef, so the counter no longer builds one record per edge or promoted posting list it passes.
  • A crash test for checkpoints that overlap commits (from Keep commits fast while compaction runs #76). A writer commits while the test checkpoints every 20 ms, the engine stops without a clean shutdown, and every commit has to come back with the counts matching the trees.

Not carried over

#76 also moved the catalog write and the WAL sync of a checkpoint out of the commit lock. #81 already syncs the segments before taking the lock, but the catalog publish and WAL sync still run under it. While writing the test I saw what that costs: with checkpoints running back to back, a writer gets very few commits in. Doing that part again on top of the reference count journal is a separate change.

Testing

  • Full suite: 155 tests pass.
  • New AtomCodecTest and PostingsCodecTest check that mapRefs keeps the same record when nothing changes and that forEachRef sees the same references mapRefs does.
  • CrashRecoveryTest.checkpointsTakenWhileCommitsRunLoseNothing runs in both WAL modes, about 13 s together.

Each inline posting list went through a stream, a list that allows
nulls and then a second copy in Postings.Inline. It is now built once
with List.of, which Inline keeps as it is.
Codecs that hold tree references only had mapRefs, which builds a new record
even when every reference maps to itself. The reference counter walks every
leaf value of every commit's changed nodes this way, and so does the check
that sets a leaf's flag for page references. mapRefs now returns the same
record when nothing changed, and a new forEachRef visits the references
without building anything; the counter and the flag check use it.
…hing

A writer commits while the test thread checkpoints every 20 ms, then the
engine stops without a clean shutdown. Every commit has to come back, with the
reference counts matching the trees. The test comes from #76, whose compaction
changes #81 replaced.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant