Repository navigation
Read ahead one page in full-log ScanCursor iterations - #2177
Closed
Shivam Sharma (ShivamSharma43) wants to merge 1 commit into
Closed
Shivam Sharma (ShivamSharma43) wants to merge 1 commit into
Shivam Sharma (ShivamSharma43) wants to merge 1 commit into
Conversation
ScanCursor always used SinglePageBuffering, so a whole-log lookup scan (DBSIZE, INFO KEYSPACE, KEYS, slot deletion) waited for each on-disk page to load before processing it, then waited again for the next one. Use DoublePageBuffering when the caller asks for the whole log (count == long.MaxValue) so the next page is read while the current page's records are liveness-checked. Bounded-count calls such as SCAN keep SinglePageBuffering, since they create a new iterator per call and would waste the read-ahead. DBSIZE with --storage-tier, 64 MB memory, 4 MB pages, log almost all on disk (Windows, NVMe), alternating A/B runs: - 60K keys x 128 KB (7.8 GB log): ~9.8 s -> ~6.8 s median - 4M keys x 64 B (316 MB log): ~2.0 s -> ~1.8 s median Refs microsoft#1436 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Ted Hart (TedHartMS)
self-requested a review
September 28, 2026 15:56
Contributor
|
Thanks for the contribution! This has been consolidated into PR #2181 |
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.
Refs #1436
Problem
DBSIZE(andINFO KEYSPACE,KEYS, and slot deletion) runs a lookup-basedScanCursorover the whole log.ScanCursoralways built its iterator withDiskScanBufferingMode.SinglePageBuffering, so for the on-disk part of the log each page was read, processed, and only then was the next page's read issued. Disk IO and the per-record liveness checks never overlapped.Change
ScanCursor(inSpanByteAllocatorImplandObjectAllocatorImpl) now usesDoublePageBufferingwhen the caller asks for the whole log (count == long.MaxValue), so the next page is read while the current one is processed. Bounded-count calls such asSCANkeepSinglePageBuffering: they create a new iterator per call and usually use only part of one page, so read-ahead would be wasted IO.Measurements
DBSIZEagainstGarnetServerwith--storage-tier --memory 64m --page 4m --index 64m/256m --no-obj, recovered from a checkpoint so the log is almost entirely belowHeadAddress. Windows 11, consumer NVMe (KIOXIA KBG40). Old and new builds alternated between runs to reduce noise:I also tried 4 frames of read-ahead; it was no better than 2 on this hardware, so this PR uses the existing
DoublePageBufferingmode.Scope relative to #1436
This does not make
DBSIZEO(1). With the storage tier enabled it is still a full scan whose cost is proportional to the log size (in the issue's setup, 256 KB values stored inline, it is mostly a sequential read of the whole log). An exact running key count seems hard to maintain, because a blind upsert on a key whose previous version is on disk does not know whether the key already existed. Happy to follow up if you'd prefer an approximate or cachedDBSIZEfor storage-tier deployments.The
INFOkeyspace part of the issue appears to be covered already by #1919 (INFO KEYSPACE, which is deliberately excluded from plainINFO).Testing
Tsavorite.test.hlogandTsavorite.test.recovery, filtered toIteration|Scan: 81/81 and 77/77 passed (15 skipped as before). These includeScanCursor(count: long.MaxValue)over fully evicted multi-page logs, with and without forced hash collisions.Garnet.testfiltered toRespScanCommandsTests|RespInfoTests|DbSize|Keys|RespRecoveryFailOpenTests: 118/118 passed.dotnet format whitespace --verify-no-changesis clean for the changed files.🤖 Generated with Claude Code