Pass reserver_ids to SQLite to pre-filter the queried chunks#138
Open
ReinierMaas wants to merge 3 commits into
Open
Pass reserver_ids to SQLite to pre-filter the queried chunks#138ReinierMaas wants to merge 3 commits into
reserver_ids to SQLite to pre-filter the queried chunks#138ReinierMaas wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a SQLite user-defined function (UDF) to let SQLite pre-filter out already-reserved chunks during chunk selection, leveraging in-process FFI to consult the Rust Reserver state while the query executes.
Changes:
- Add
opsqueue_is_reserved(submission_id, chunk_index)filtering to allStrategychunk-selection queries and register the UDF on SQLite connections before fetching. - Expose
Reserver::is_reservedand implement the SQLite UDF callback + destructor wiring (including tests verifying reserved chunks are excluded). - Minor supporting fixes:
lookup_id_by_prefixparameter cleanup andChunkIndex: TryFrom<i64>for converting SQLite integers safely.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| opsqueue/src/consumer/strategy.rs | Adds opsqueue_is_reserved(...) = 0 filters to strategy queries; updates query-plan tests with a no-op UDF registration. |
| opsqueue/src/consumer/dispatcher/reserver.rs | Adds Reserver::is_reserved used by the SQLite UDF callback. |
| opsqueue/src/consumer/dispatcher/mod.rs | Registers the SQLite UDF per acquired reader connection; implements the FFI callback/destructor; adds an integration-style test asserting reserved chunks are excluded. |
| opsqueue/src/common/submission.rs | Simplifies lookup_id_by_prefix to reuse $1 and avoids redundant bindings. |
| opsqueue/src/common/chunk.rs | Adds TryFrom<i64> for ChunkIndex to support parsing SQLite int64 values in the UDF. |
| opsqueue/Cargo.toml | Adds optional libsqlite3-sys dependency gated behind server-logic. |
| Cargo.lock | Records the new libsqlite3-sys dependency resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
This uses the fact that SQLite query engine lives in the same process as the calling Rust code to pass the integers directly into the query via SQL-FFI.
This can be extended to the
metastateas well so we don't need to serialize to JSON in order to use the data in the query itself.