chore(server): Introduce read budget to prep for tagged chunk loading#7109
Conversation
abhijat
changed the title
There was a problem hiding this comment.
Pull request overview
Prepares the RDB loader/saver pipeline for upcoming “tagged chunk” loading (PR #7070) by introducing chunk budget accounting hooks and refactoring loader dispatch so value loading can later stop/resume at chunk boundaries.
Changes:
- Introduce chunk budget/state plumbing in
RdbLoaderBaseand refactorLoadKeyValPairinner loop intoReadAndDispatchObject. - Update SBF loading to support split/partial filter reads (to work with filter chunking and future tagged chunking).
- Prevent stream serialization from flushing between the final listpack and the stream metadata tail.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/server/rdb_save.cc | Avoids flushing after the last stream listpack so tail metadata stays bundled. |
| src/server/rdb_load.h | Adds chunk-budget state/hooks and SBF partial-filter continuation state; refactors chunked-value tracking key. |
| src/server/rdb_load.cc | Implements budget consumption helpers, refactors load dispatch, and extends SBF loading for partial filter reads. |
| src/server/rdb_extensions.h | Adds opcode constant for upcoming tagged chunk support. |
🤖 Augment PR SummarySummary: This PR prepares the RDB loader for upcoming “tagged chunk” loading by introducing a (currently no-op) per-chunk read budget and plumbing partial-read boundaries through the object-loading loop. Changes:
Technical Notes: The chunk state and budget remain effectively unlimited in this PR; the new checks are intended to be activated by a follow-up that parses tagged chunk headers and sets per-chunk limits. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
abhijat
force-pushed
the
abhijat/chore/prep-chunk-load
branch
2 times, most recently
from
642e835 to
423b5c5
Compare
| if (n > current_chunk_state_->remaining_payload_bytes) | ||
| return RdbError(errc::rdb_file_corrupted); |
There was a problem hiding this comment.
I would advise some log here or in the call chains because it will be very difficult to debug an error with rdb_file_corrupted and no other data. Alternatively we can split rdb_file_corrupted into multiple corrupted codes
There was a problem hiding this comment.
I added a log and also added new error codes for short and over read, as the next PR has more such places.
abhijat
force-pushed
the
abhijat/chore/prep-chunk-load
branch
from
767d29d to
93195d6
Compare
Skip PushToConsumerIfNeeded after the last listpack in SaveStreamObject. The stream metadata tail (length, IDs, consumer groups) must stay bundled with the last listpack chunk so the loader can read it in the same chunk.
abhijat
force-pushed
the
abhijat/chore/prep-chunk-load
branch
from
d4ccedc to
c9d40d4
Compare
This PR is a preparatory step towards #7070 to make the diff smaller and easier to review. The PR produces no functional changes.
The main change introduced here is a chunk state. This remains unset in this PR.
A chunk budget is also introduced. This budget is to be read from a tagged chunk header. This budget also remains unlimited in this PR.
Essentially the budget checks here are no-ops, but they prepare for the next PR where tagged chunk header will be read, the budget will be set and the read functions will return at exact chunk boundaries.
Two other changes introduced here:
ReadAndDispatchObjectintroduced. It makes the inner loop ofLoadKeyValPairisolated, which will be reused in later PR feat(server): Implement tagged chunk loading #7070 to load chunks of a value.