[InvocationStatus] Add exact ID selection and batched multi-get point reads#5046
[InvocationStatus] Add exact ID selection and batched multi-get point reads#5046AhmedSoliman wants to merge 2 commits into
Conversation
Test Results 8 files 8 suites 4m 52s ⏱️ Results for commit 2ec43b0. ♻️ This comment has been updated with latest results. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5754ad5895
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Some(selection) if selection.keys.len() > 4096 => { | ||
| itertools::Either::Left(Some((partition_id, partition)).into_iter()) | ||
| } |
There was a problem hiding this comment.
For sys_invocation_status, with_grouped_invocation_id("id") enables PerPartition fanout, but this guard runs before the PerPartition arm, so an id IN (...) with more than 4096 distinct partition keys schedules every live partition instead of only partitions containing requested IDs. On partitions with no matching ID, InvocationIdFilter::new returns no ID selection and the status table converts that to a PartitionKey scan, causing unnecessary full partition scans; the per-key safety limit should not disable the grouped path because it creates at most one scan per Restate partition.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| /// Maximum number of invocation-status keys passed to one RocksDB multi-get call. | ||
| const INVOCATION_STATUS_MULTI_GET_BATCH_SIZE: usize = 500; |
There was a problem hiding this comment.
My concern here is that invocation status can be large (if completed with big payloads) and without a good way to limit this we will blow up memory.
59848ee to
cf00086
Compare
Replace the specialized partition and key-range scan variants with a compact TableScan API that lowers encoded bounds to prefix or exclusive range scans. Use prefix seeking only when the encoded bounds remain within RocksDB's fixed prefix, fall back to total-order seeking for cross-prefix ranges, and turn singleton ranges into prefix scans. Centralize iterator-bound configuration across direct, background, and transactional readers. Let callers supply ReadOptions so async I/O and transaction snapshots remain caller policy, while short prefixes safely bypass the fixed-prefix extractor and its bloom filters. Remove the iterator wrapper plumbing, share encoded Bytes bounds across RocksDB paths, and migrate table and storage-migration callers to the consolidated API. Cover inclusive upper bounds, singleton partition ranges, and prefix-versus-total-order selection.
… reads Keep equality and IN predicates as exact ID selections in the DataFusion filter layer instead of collapsing large sets into scan ranges. This lets point-lookup tables preserve the user's requested IDs while range-scan tables can still convert the same selection back to bounds. Teach the partitioned table provider to group exact point reads by Restate partition when the scanner can re-fetch the exact IDs itself. Use that for sys_invocation_status by adding an InvocationIdSet scan path backed by bounded RocksDB multi-get batches. Split partition-store inclusive key-range scans into single-prefix and total-order variants, and use total-order scans when invocation-status, journal, or journal-events ID bounds span multiple partition keys. This preserves correctness for range-scan consumers of exact ID selections. Adapt the worker cleaner to the fallible stream shape used by the new invocation-status scan path, and add scan-mode coverage for the new total-order range variant. This design delivers 5-10x better performance for point-lookups than the concurrent iterator-per-entry approach that was previously used. This pays more dividends on slower disks and will allow us to leverage async I/O better when we build rocksdb with folly.
1 participant
Keep equality and IN predicates as exact ID selections in the DataFusion filter layer instead of collapsing large sets into scan ranges. This lets point-lookup tables preserve the user's requested IDs while range-scan tables can still convert the same selection back to bounds.
Teach the partitioned table provider to group exact point reads by Restate partition when the scanner can re-fetch the exact IDs itself. Use that for sys_invocation_status by adding an InvocationIdSet scan path backed by bounded RocksDB multi-get batches.
Split partition-store inclusive key-range scans into single-prefix and total-order variants, and use total-order scans when invocation-status, journal, or journal-events ID bounds span multiple partition keys. This preserves correctness for range-scan consumers of exact ID selections.
Adapt the worker cleaner to the fallible stream shape used by the new invocation-status scan path, and add scan-mode coverage for the new total-order range variant.
This design delivers 5-10x better performance for point-lookups than the concurrent iterator-per-entry approach that was previously used. This pays more dividends on slower disks and will allow us to leverage async I/O better when we build rocksdb with folly.
Stack created with Sapling. Best reviewed with ReviewStack.