Add basic kRoundRobin compaction policy

[littlepig2013]

Summary:

Add kRoundRobin as a compaction priority. The implementation is as follows.

• Define a cursor as the smallest Internal key in the successor of the selected file. Add vector<InternalKey> compact_cursor_ into VersionStorageInfo where each element (InternalKey) in compact_cursor_ represents a cursor. In round-robin compaction policy, we just need to select the first file (assuming files are sorted) and also has the smallest InternalKey larger than/equal to the cursor. After a file is chosen, we create a new Fsize vector which puts the selected file is placed at the first position in temp, the next cursor is then updated as the smallest InternalKey in successor of the selected file (the above logic is implemented in SortFileByRoundRobin).
• After a compaction succeeds, typically InstallCompactionResults(), we choose the next cursor for the input level and save it to edit. When calling LogAndApply, we save the next cursor with its level into some local variable and finally apply the change to vstorage in SaveTo function.
• Cursors are persist pair by pair (<level, InternalKey>) in EncodeTo so that they can be reconstructed when reopening. An empty cursor will not be encoded to MANIFEST

Test Plan: add unit test (CompactionPriRoundRobin) in compaction_picker_test, add kRoundRobin priority in CompactionPriTest from db_compaction_test, and add PersistRoundRobinCompactCursor in db_compaction_test

[ajkr]

Do you mind reminding me why we thought the compaction cursor was updated too early? Was there some test failure (perhaps triggered by an additional code change)?

[littlepig2013]

Do you mind reminding me why we thought the compaction cursor was updated too early? Was there some test failure (perhaps triggered by an additional code change)?

Oh, there was no testing failure on that. The motivation is that we want to update the cursor only when the compaction succeeds (suppose that it selects the first file), while the current implementation just blindly updates (advances) the cursor no matter whether the following compaction succeeds or not. If we want to implement this, we may need to update the cursor here (after Run() status is ok)

rocksdb/db/db_impl/db_impl_compaction_flush.cc

Line 3406 in ff32346

*c->mutable_cf_options());

[jay-zhuang]

A few design questions:

• I think we should only update cursor when the compaction is done;

• for one level, only one compaction (the one starts from the cursor) is able to update the cursor when it's done, which can be blocked by:

1. there's already another compaction starts from that level, or
2. a high level compaction, for example, in your "compacting" test case, L1 file [100, 240] is compacting L2 which overlaps with current cursor;
   In both cases, the "next" file may likely also be overlapping with the ongoing compaction, in the following example, if T1 is compacting to the next level, T2 is the next file, which is unable to compact, because the next level file is compacting:
   
   So we could continue select the next next file T3, until we find one. Or another strategy is just fall back to kMinOverlappingRatio, because anyway these compaction won't update cursor (hopefully these compactions are only happening because of high compaction pressure)

• ideally, we should use subcompaction to increase the compaction parallelism instead of having multiple compactions, but deciding how many input files (subcompaction number) is challenge, as if we choose too many it may delay other levels compaction. We don't need to handle it for now, it will be the next step for the project.

[ajkr]

I think we should only update cursor when the compaction is done;

I may have suggested the opposite to Zichen. Do we know the compaction reason at compaction done time? We only want to advance for CompactionReason::kLevelMaxLevelSize compactions not all compactions. This is known when we pick the compaction and it sounds like we don't plan to use the cursor again until after that compaction finishes regardless of where the update happens. The only difference then is whether the advance happens in case compaction fails. I am neutral on this matter so prefer whatever's simpler to implement. Note that LevelDB actually considered it a benefit to advance the cursor despite failure -- https://github.com/google/leveldb/blob/d019e3605f222ebc5a3a2484a2cb29db537551dd/db/version_set.cc#L1440-L1445.

[ajkr]

This is known when we pick the compaction and it sounds like we don't plan to use the cursor again until after that compaction finishes regardless of where the update happens

Actually I'd take that back and guess it's OK to let picking try the next file. It'll almost certainly conflict due to output-level files in common with the concurrent compaction. But it doesn't seem bad to let it be picked if it somehow works out.

[ajkr]

Thanks for the progress. Seems we have a good grasp of the approach now. Had some suggestions for finishing this.

I think we should call SetCompactCursor() when constructing the VersionEdit for installing a compaction result. I found three places where that happens:

• InstallCompactionResults()
• BackgroundCompaction()'s c->deletion_compaction() branch
• BackgroundCompaction()'s c->IsTrivialMove() branch

The intuition for updating at this layer is we have the relevant info like knowledge of compaction input level (we only want to advance the input level's cursor), and compaction reason may be useful too. In the initial implementation we could always advance input-level cursor when reason is CompactionReason::kLevelMaxLevelSize, input level is one or higher, and compaction priority is kRoundRobin.

[ajkr]

Do we intend to remove these early returns eventually? If so perhaps add TODOs.

[littlepig2013]

I think we should call SetCompactCursor() when constructing the VersionEdit for installing a compaction result. I found three places where that happens:

• InstallCompactionResults()
• BackgroundCompaction()'s c->deletion_compaction() branch
• BackgroundCompaction()'s c->IsTrivialMove() branch

For this condition, BackgroundCompaction()'s c->deletion_compaction() branch, I noticed that it is only for kCompactionStyleFIFO. Does this also apply to level-based round-robin? If not, we may omit advancing the cursor there?

[ajkr]

For this condition, BackgroundCompaction()'s c->deletion_compaction() branch, I noticed that it is only for kCompactionStyleFIFO. Does this also apply to level-based round-robin? If not, we may omit advancing the cursor there?

Oh ok. Yeah it does not apply so is fine to omit.

[ajkr]

Looks good, few more things.

[ajkr]

LGTM!!

[facebook-github-bot]

@littlepig2013 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[ajkr]

LGTM, feel free to land.