-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor archiving code for incremental mapping generation #3100
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Areas of the code where I'd like some extra review
// We can't tell the difference between an archiver initialized with a | ||
// block continuation, and a spill over from a previous segment. So the | ||
// only way to handle both cases without duplication is to skip a trailing | ||
// continuation, then produce mappings for that continuation when the next | ||
// block is added. (Either here or when the segment is full.) | ||
[.., SegmentItem::BlockContinuation { .. }] => &[], | ||
// If we've just finished the previous segment, the only item in the buffer | ||
// is a SegmentHeader, so there are no new blocks to produce mappings for. | ||
[SegmentItem::ParentSegmentHeader(_)] => &[], | ||
_ => unreachable!("Caller always adds a new block; qed. {}", segment), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all the remaining cases really unreachable?
If we're not absolutely sure, I'm happy to make all these patterns skip the mappings (rather than panicking).
// No new blocks, unreachable as long as the caller added a new block | ||
// (or we're still processing a large block continuation) | ||
_ => { | ||
unreachable!( | ||
"Caller always adds a new block, or previous call adds block continuation; qed. {}", | ||
segment, | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar question here: are we absolutely sure there are no other valid patterns?
66dda15
to
545ce28
Compare
This PR performs object mapping generation at the end of most blocks, rather than only at the end of a full segment.
This reduces load bursts on the mapping indexer, and reduces object submission to mapping latency.
Piece generation still requires a full segment.
Code contributor checklist: