feat(oracle): page interval indexes beyond the 1024-record cap#141
feat(oracle): page interval indexes beyond the 1024-record cap#141faddat wants to merge 4 commits into
Conversation
Replace the monolithic per-bucket Vec index with paged storage so high-volume namespaces can append without IndexFull or rewriting the entire index on each write. Raise MAX_QUERY_INTERVALS so day/month-scale range queries remain usable. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR upgrades the oracle’s interval indexing scheme to use paged storage (1024 record ids per page) so interval buckets can grow beyond the prior 1024-id cap without rewriting the entire index on each append, and increases MAX_QUERY_INTERVALS to support much larger inclusive interval range queries.
Changes:
- Introduces paged interval index metadata (
IntervalIndexMeta) and the newINDEX_PAGE_SIZEconstant (withMAX_RECORDS_PER_BUCKETretained as an alias). - Reworks oracle DB indexing to store per-(namespace|writer, interval) index pages plus per-index meta (breaking storage layout: table tags 4/5 become meta, pages use 6/7).
- Adds tests covering multi-page indexing and large interval range query behavior; increases
MAX_QUERY_INTERVALSto 100_000.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| oracle/src/types.rs | Adds INDEX_PAGE_SIZE and IntervalIndexMeta to support paged interval indexes. |
| oracle/src/db.rs | Implements paged index meta + pages, and new append-based index updates. |
| oracle/src/ledger.rs | Switches record appends to use DB index append operations (instead of rewriting full index vectors). |
| oracle/src/lib.rs | Re-exports new paging types/constants from the crate API. |
| oracle/src/tests/mod.rs | Adds multi-page and large-range query test cases; updates imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self.db.set_record(&record); | ||
|
|
||
| namespace_records.push(id); | ||
| self.db | ||
| .set_namespace_index(namespace, interval, &namespace_records); | ||
|
|
||
| writer_records.push(id); | ||
| self.db.set_writer_index(signer, interval, &writer_records); | ||
| .append_namespace_index(namespace, interval, id) | ||
| .await?; | ||
| self.db.append_writer_index(signer, interval, id).await?; |
There was a problem hiding this comment.
Rejecting this suggestion.
set_record is infallible; the only fallible steps are the two index appends. Writing the record first is intentional: if an index append fails, a retry with the same nonce overwrites the same record id and can finish indexing. Index-then-record would leave namespace queries pointing at missing payloads (MissingRecord), which is worse.
Also, on the chain path a failed apply_transaction aborts block application and the dirty state is discarded—same pattern as the pre-paging oracle code. IndexFull is now only reachable on page_count overflow, not routine appends.
| let mut records = Vec::new(); | ||
| for page in 0..meta.page_count { |
| let mut records = Vec::new(); | ||
| for page in 0..meta.page_count { |
| StateStore::set( | ||
| self, | ||
| namespace_interval_key(namespace, interval), | ||
| encoded(&records.to_vec()), | ||
| namespace_meta_key(namespace, interval), | ||
| encoded(&next_meta), | ||
| ); |
There was a problem hiding this comment.
Already fixed in 174dd5f — meta is only written when next_meta != meta.
| writer_page_key(writer, interval, page_index), | ||
| encoded(&page), | ||
| ); | ||
| StateStore::set(self, writer_meta_key(writer, interval), encoded(&next_meta)); |
There was a problem hiding this comment.
Already fixed in 174dd5f — meta is only written when next_meta != meta.
| .map_err(|err| OracleError::Storage(err.to_string()))? | ||
| { | ||
| Some(bytes) => records.extend(decode_page(&bytes)?), | ||
| None => return Err(OracleError::MissingRecord), |
There was a problem hiding this comment.
Fixed in 3b4cea9 — missing index pages now return OracleError::MissingIndex.
| .map_err(|err| OracleError::Storage(err.to_string()))? | ||
| { | ||
| Some(bytes) => decode_page(&bytes)?, | ||
| None => return Err(OracleError::MissingRecord), |
There was a problem hiding this comment.
Fixed in 3b4cea9 — missing index pages now return OracleError::MissingIndex.
| .map_err(|err| OracleError::Storage(err.to_string()))? | ||
| { | ||
| Some(bytes) => records.extend(decode_page(&bytes)?), | ||
| None => return Err(OracleError::MissingRecord), |
There was a problem hiding this comment.
Fixed in 3b4cea9 — missing index pages now return OracleError::MissingIndex.
| .map_err(|err| OracleError::Storage(err.to_string()))? | ||
| { | ||
| Some(bytes) => decode_page(&bytes)?, | ||
| None => return Err(OracleError::MissingRecord), |
There was a problem hiding this comment.
Fixed in 3b4cea9 — missing index pages now return OracleError::MissingIndex.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
oracle/src/db.rs:236
append_namespace_indexalways rewrites the meta entry even whenpage_countis unchanged. The PR description states meta should only be updated when allocating a new page; skipping redundant meta writes reduces storage churn in hot append paths.
StateStore::set(
self,
namespace_meta_key(namespace, interval),
encoded(&next_meta),
);
oracle/src/db.rs:300
append_writer_indexalways rewrites the meta entry even whenpage_countis unchanged. This adds an extra write per append; meta only needs to be updated when allocating a new page (whenpage_countchanges).
StateStore::set(self, writer_meta_key(writer, interval), encoded(&next_meta));
| fn decoded<T: Read<Cfg = ()>>(bytes: &[u8]) -> Result<T, OracleError> { | ||
| let mut buf = bytes; | ||
| T::read(&mut buf).map_err(|err| OracleError::Storage(err.to_string())) | ||
| } |
There was a problem hiding this comment.
Fixed in 3b4cea9 — decoded / decode_page now reject trailing bytes, so a legacy Vec-encoded index blob cannot be partially interpreted as IntervalIndexMeta. Added unit tests for both paths.
…a writes Add MAX_QUERY_RECORDS so range queries fail instead of buffering unbounded results, enforce the limit while paging index ids, and only rewrite interval index meta when page_count changes. Co-authored-by: Cursor <cursoragent@cursor.com>
Reject trailing bytes when decoding stored values so legacy index blobs cannot be misread as IntervalIndexMeta, return MissingIndex when an index page is absent, and pre-reserve page-load capacity for range queries. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
oracle/src/db.rs:206
IntervalIndexMetais decoded from table tag 4/5 without validating the byte length. If an operator accidentally runs against pre-migration state (where tag 4/5 stored a Vec), the first 4 bytes will be interpreted aspage_count, potentially leading to very large reads and confusingMissingRecorderrors instead of a clear storage-layout mismatch. Consider rejecting any meta value that is not exactly one u32.
fn set_record(&mut self, record: &OracleRecord) {
let key = NS.key(Table::Record, record.id.encode().as_ref());
StateStore::set(self, key, encoded(record));
}
oracle/src/db.rs:235
- Same as
namespace_index: meta decoding should validate the stored value is actually anIntervalIndexMeta(4 bytes) so an old-format index (pre-migration) or corrupted value fails fast with a clearStorageerror, rather than being misinterpreted as a boguspage_count.
{
Some(bytes) => {
extend_within_limit(&mut records, decode_page(&bytes)?, max_records)?
}
None => return Err(OracleError::MissingIndex),
oracle/src/db.rs:279
IntervalIndexMetais decoded without validating the stored byte length. To avoid silently misinterpreting pre-migration state (or corrupted values) as a largepage_count, validate that the meta value is exactly one u32 and return a clearStorageerror otherwise.
if next_meta != meta {
StateStore::set(
self,
namespace_meta_key(namespace, interval),
encoded(&next_meta),
oracle/src/db.rs:308
- Same as
writer_index: validate meta encoding length before decoding. This makes failures on pre-migration/corrupted state deterministic and easier to diagnose.
for page in 0..meta.page_count {
match StateStore::get(self, &writer_page_key(writer, interval, page))
.await
.map_err(|err| OracleError::Storage(err.to_string()))?
{
| let meta_keys_after_first = after_first | ||
| .values | ||
| .iter() | ||
| .filter(|(_, value)| value.is_some()) | ||
| .count(); | ||
|
|
||
| let mut ledger = OracleLedger::new(after_first); | ||
| ledger | ||
| .apply_transaction( | ||
| &append_tx(&writer, 1, namespace(), 4, b"second".to_vec()), | ||
| context(1_100), | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
| let after_second = ledger.into_inner(); | ||
| let meta_keys_after_second = after_second | ||
| .values | ||
| .iter() | ||
| .filter(|(_, value)| value.is_some()) | ||
| .count(); | ||
|
|
||
| // Second append fits on the existing page: one new record value, no new | ||
| // index meta key, and the page key is overwritten in place. | ||
| assert_eq!(meta_keys_after_second, meta_keys_after_first + 1); |
| /// | ||
| /// Sized to allow month-scale day buckets and day-scale minute buckets without | ||
| /// forcing consumers to shard interval keys. | ||
| pub const MAX_QUERY_INTERVALS: u64 = 100_000; |
There was a problem hiding this comment.
🟨 Range query can trigger up to 100k sequential storage reads per call
MAX_QUERY_INTERVALS is raised from 1024 to 100_000 (oracle/src/types.rs:25). Each helper range query loops one bucket at a time (for bucket in start.bucket..=end.bucket at oracle/src/ledger.rs:116 and :140), issuing at least one async StateStore::get per bucket for the index meta. A caller who supplies a wide interval range (even over entirely empty buckets) forces up to 100,001 sequential storage lookups per query, a 100x increase in worst-case read work over the previous cap. If records_by_namespace/records_by_writer are reachable from untrusted RPC/query callers, this is a resource-exhaustion (DoS) amplification vector.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
oracle/src/tests/mod.rs:417
- These variables are named
meta_keys_*but they actually count all keys withSome(value)in the in-memory store (records, pages, and meta). Renaming them would make the test intent clearer and avoid implying the count is limited to meta keys.
let meta_keys_after_first = after_first
.values
.iter()
.filter(|(_, value)| value.is_some())
.count();
| if records.len().saturating_add(page.len()) > max_records { | ||
| return Err(OracleError::InvalidQuery( | ||
| "query result exceeds MAX_QUERY_RECORDS", | ||
| )); | ||
| } |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
(namespace|writer, interval)Vec<RecordId>index with paged storage (INDEX_PAGE_SIZE = 1024ids/page). Appends only rewrite the last page (+ meta when allocating a new page), so high-volume buckets are no longer capped at 1024 records and no longer rewrite the full index on every write.MAX_QUERY_INTERVALSfrom 1024 → 100_000 so day/month-scale range queries work without forcing consumers to hash-shard interval keys.MAX_RECORDS_PER_BUCKETas a backward-compatible alias ofINDEX_PAGE_SIZE(documented as page capacity, not a hard bucket ceiling).IndexFullis retained only forpage_countoverflow.Breaking storage layout for oracle indexes (table tags 4/5 become meta; pages use 6/7). Existing oracle state must be reset/migrated.
Motivated by Knight ingest of QueenOne
event_agg.sends(~700k rows/day, peak ~20k/min), which cannot fit the old hard cap.Test plan
cargo test -p nunchi-oracle(9 tests, including new multi-page + large-range cases).knight-chain)Made with Cursor