fix: measured vector conformance gaps, and the index-build lifecycle as shared code - #306
Merged
LeeroyHannigan merged 19 commits intoAug 27, 2026
Merged
Conversation
8 tasks
yesyayen
force-pushed
the
feat/vector-shared-groundwork
branch
from
August 25, 2026 03:04
3a3cd5b to
08ad04d
Compare
yesyayen
marked this pull request as ready for review
August 25, 2026 16:21
yesyayen
requested review from
LeeroyHannigan,
amrith,
c33howard,
jcshepherd,
pdf-amzn and
robinnsc
as code owners
August 25, 2026 16:21
LeeroyHannigan
left a comment
LeeroyHannigan
left a comment
Collaborator
There was a problem hiding this comment.
Nice work, the shared lifecycle module and pinning strings to real captures is the right call. Two things holding it up, both from the branch being cut before the vector fixes landed on main. Rebase onto current main and re-apply the extraction on top, and I think it's most of the way there.
| /// [`INVALID_PARAMETER_VALUES_ONCE`] instead, so the envelope goes with the error | ||
| /// KIND rather than with this file. Reading the four captures above as one rule | ||
| /// is what got it wrong once already: they are all doubled-envelope kinds. | ||
| const INVALID_PARAMETER_VALUES_TWICE: &str = |
Collaborator
There was a problem hiding this comment.
Blocking - the doubled-vs-single split isn't per error kind, it's per region. us-east-1 and us-west-2 give the single envelope for every kind; eu-west-1 gives the doubled one for every kind. So these byte-exact tests match no single region. Pick one region's shape and use it for all four kinds.
Collaborator
Author
There was a problem hiding this comment.
good catch - re-probed all 3 regions and you're right, it's per region not per kind. us-east-1/us-west-2 single, eu-west-1 doubled.
pinned everything to the us shape in e5af651 commit. Two message bodies also changed since our original capture (type error is prose now, size error dropped a period) so those got pinned too.
| /// 2026-08-19). Carries the whole message because the state, and therefore | ||
| /// the wording, is known only to the backend that holds it. | ||
| #[error("{0}")] | ||
| ResourceInUse(String), |
Collaborator
There was a problem hiding this comment.
Should-fix - this overlaps IndexesInUse already on main (both map to ResourceInUseException) and has no producer yet. Worth folding into IndexesInUse on the rebase rather than carrying two.
Collaborator
Author
There was a problem hiding this comment.
Yep, agreed - both mapped to the same exception and ours had no producer left after the rebase. Folded it into IndexesInUse. The variant now carries the full message composed by the backend, and the engine just maps it to ResourceInUseException as-is. The delete-table sentence moved into its producer unchanged so nothing moved on the wire.
Unit tests cover the passthrough and pin the allocation-phase string to the probe capture, and the wire suite exercises both refusals end to end.
|
|
||
| async fn set_backfilling(&mut self) -> Result<(), StorageError> { | ||
| sqlx::query( | ||
| "UPDATE vector_indexes SET backfilling = 1 WHERE table_id = ? AND index_id = ?", |
Collaborator
There was a problem hiding this comment.
Blocking - this is the pre-fix backfill. Rebasing needs to keep main's KeepExisting/INSERT OR IGNORE path and the min-CREATING window - both are dropped as the branch stands, so a write landing mid-backfill can wedge the build again.
Collaborator
Author
There was a problem hiding this comment.
yes, the extraction was based on a pre-#304 snapshot so both would've been dropped.
Fixed it during the rebase. The KeepExisting/INSERT OR IGNORE path is back in the backfill apply hook, and the min-CREATING window moved into the shared complete_build (new earliest_active param, repair paths skip it).
Probes P4, P5, P9 and P10 captured the whole strings byte-exact from real Amazon DynamoDB on 2026-08-19. Three templates diverged: the leading sentence was missing, so was the space before the second full stop, the size message lost the stop after the actual count, and the attribute-type message named a prose type instead of the DynamoDB type tokens.
One shared envelope constant now carries the service's doubled sentence and its space before the full stop, and the attribute-type refusal reports Expected: L with the actual type token.
Probe P8 captured both members on every SearchVectors response, always equal, under INDEXES and TOTAL alike; ExtendDB emitted only the bytes member, so a client reading units got null. Constructors set the pair together so the two cannot drift. The write side keeps its single measured member: no capture exists for a write-side units member.
…Table UpdateTable had no Unsupported arm, so a backend that cannot serve a request answered 500 where CreateTable answered 400 for the same refusal. A backend whose capability is decided at runtime needs that arm. ResourceInUse carries the state-dependent refusal the service returns when a vector index is deleted during resource allocation (probe P2, whole string in a core builder). Plumbing only: no path constructs it yet. The UpdateTable error map moves into a named function so every arm, including the two no in-tree backend can produce, is unit testable.
Both create-path messages were inlined although the constants existed, one of them with no users at all, which is why the enforcement was hard to find. The count constant spells its limit out, so a test now holds it to the limit constant.
…s provisioned Only the opposite direction was guarded, a switch to PROVISIONED while indexes exist. Probe P12 measured the add direction returning the same whole string, and measured that the check is on the request's net state: a switch to PAY_PER_REQUEST and a create in one call is accepted.
Four tests, each asserting whole strings or the whole member set: the SearchVectors capacity shape under INDEXES and TOTAL, the search-path invalid-values message for all three causes, the write-path size, type and range refusals across PutItem and UpdateItem, and both directions of the net-state billing rule for a vector index create. Verified failing against main first, over the wire: the capacity response carried one member, the two messages were truncated, and a vector index create on a PROVISIONED table returned 200 with the index CREATING.
Probe P5: a vector-semantic failure inside TransactWriteItems cancels per item, with the failing item's Message equal to the PutItem refusal. The shared validator makes that automatic; nothing asserted it.
Probe P13 measured a wrong-typed element inside the vector list returning "One or more parameter values were invalid. " with an ordinary full stop, not the doubled envelope. The envelope varies by error kind, not by nesting level: attribute-level size and type and element-level range are doubled, element-level type is not. BOOL is asserted alongside S because it shows the type token is not limited to the key types.
Probe P13 measured this one kind with an ordinary full stop and no leading sentence. The two envelopes are now separate constants named for which is which, since the discriminator is the error kind and reading four doubled-envelope captures as one rule is what got it wrong.
Extract the backend-agnostic half of the vector index-build lifecycle into extenddb-storage, ahead of a second backend implementing VectorSearchEngine. ADR-0005 forbids that backend re-implementing the lifecycle; this module is the mandated extraction target. Carries the measured status sequence and its ordering rules (complete_build, rebuild_index, run_backfill over the VectorIndexBuild primitives), the poison-row classification, the shared row-shape helpers (partition_value, UNSCOPED_PARTITION, projected_payload, VectorIndexMeta, VectorApplyContext), and driver unit tests over a scripted backend. No behaviour change: nothing consumes the module yet.
… code Rewire the SQLite backend as the first implementor of the shared lifecycle (ADR-0005): SqliteVectorBuild supplies the storage primitives (rowid-cursor batch, backfilling flag, ACTIVE flip, data-table reset, worker wake), and the create path, startup reconciler, and stuck-build recovery now run through complete_build / rebuild_index instead of a local copy of the state machine. Backfill row classification, the row payload rules, partition scoping, and the queue-context types come from extenddb_storage::vector_lifecycle; their SQLite copies are deleted. Behaviour preserving: same SQL, same status sequence, same log wording, same queue bytes. Test deltas are mechanical (import paths, the batch cursor becoming Option); the two partition tests moved to the shared module with the code they pin.
The decision deferred the extraction until a second backend implemented VectorSearchEngine and made it that backend's first task. That condition arrived with the PostgreSQL port: the extraction landed, so the ADR now records the outcome instead of asserting the lifecycle still lives in storage-sqlite.
The measured refusal (probe P2) has its message constant, error variant, and engine mapping arms on this branch, but no runtime producer: the delete branch does not read the build phase yet. Enforcement lands with the shared-lifecycle follow-up; the tracker row keeps the gap visible so the plumbing is not mistaken for a closed item.
The opening row lookup now carries billing_mode, and the three billing checks (net-state vector guard, throughput-vs-mode rejection, no-op throughput rejection) read that one value instead of re-querying the same row. One read, one source of truth. No behavior change: all reads were inside one transaction on one row.
…d a pinned-vs-extrapolated token Review nits: run_backfill's doc states the failure contract a direct caller takes on; rebuild_index documents that recovery does not re-assert Backfilling, matching pre-extraction behavior; the non-list vector test notes which type token is measured and which is extrapolated.
A git add -A swept the isolated test server's log into what was meant to be a docs-only commit. Remove it from tracking and ignore the whole instance directory so the database file and future logs cannot follow.
…elope Measured 2026-08-27 with raw signed requests, all four error kinds, three regions: us-west-2 and us-east-1 answer one envelope sentence for every kind, eu-west-1 answers the doubled sentence for every kind, the same day. The envelope is region-uniform, not per kind. The 2026-08-19 capture that shaped the previous strings recorded us-west-2 mid-transition, doubled for three kinds and single for the fourth, so neither a per-kind rule nor a clean deployment-wave story explains both datasets; these strings pin the us shape as measured 2026-08-27. Two bodies travelled with the envelope: the whole-attribute type error is now a prose expected type with no actual-type token, and the size error lost the full stop between the actual count and IndexName.
Main gained IndexesInUse for the delete-table refusal while this branch added ResourceInUse for the vector phase refusal, and both map to ResourceInUseException. One rule replaces them: the backend composes the whole client-facing message, because only it knows which in-use state it refused for, and the engine passes it through verbatim. The delete-table sentence moves to its producer unchanged, so the wire strings do not move.
yesyayen
force-pushed
the
feat/vector-shared-groundwork
branch
from
August 26, 2026 23:34
08ad04d to
85b8f60
Compare
yesyayen
added a commit
to yesyayen/extenddb
that referenced
this pull request
Aug 28, 2026
Replay of the feat/wasm-shared-crates gating pass onto current main: - auth takes HeaderMap from http, not axum - cache: moka/tokio native-only; wasm32 uses a pass-through SWR shim - core: uuid js + time wasm-bindgen features on wasm32 - engine: tokio native-only; import/export surface gated with it - storage: tokio/tokio-util/bcrypt/rand/aes-gcm native-only; bootstrapper, server_components, backend registry and hooks gated off wasm32 New on top of that pass (post-ExtendDB#306/ExtendDB#307 call sites): - storage vector_lifecycle build driver (tokio::time) is native-only; the pure row-shape helpers stay on both targets
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
2 participants
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.
What
Vector conformance fixes, each pinned to a wire capture from Amazon DynamoDB, plus the ADR-0005 lifecycle extraction. Both are prerequisites for a second backend implementing vector search.
ConsumedCapacityreportsVectorSearchUnitsalongsideVectorSearchRequestBytes(always equal, as the service returns them).StorageError::ResourceInUsewith its engine mapping (plumbing for the measured allocation-phase delete refusal, tracked as F-18), and the missingUnsupportedarm on UpdateTable: a backend refusal surfaced as a 500 where CreateTable answers 400.extenddb_storage::vector_lifecycleas shared drivers over one backend trait (backend-owned cursor type, defaulted heartbeat hook). Behavior-preserving: zero test-expectation changes.Behavior: today vs Amazon DynamoDB
Every value below was captured from Amazon DynamoDB via raw signed requests (us-west-2, 2026-08-19).
ConsumedCapacity{"VectorSearchRequestBytes": 1024.0}{"VectorSearchRequestBytes": 1024.0, "VectorSearchUnits": 1024.0}Search vector contains invalid valuesSearch vector contains invalid values. All values in the search vector must be a 32-bit floating-point number attributeOne or more parameter values were invalid. Invalid size for parameter emb, Expected: 4, Actual: 3. IndexName: vidxOne or more parameter values are not valid. One or more parameter values were invalid . Invalid size for parameter emb, Expected: 4, Actual: 3. IndexName: vidxValidationException: One or more parameter values were invalid: Vector indexes are only supported for PAY_PER_REQUEST tablesWhy
Conformance gaps found by comparing ExtendDB against Amazon DynamoDB. The previous strings were pinned by tests asserting the wrong text.
Closes # n/a
Testing done
Captured every expected string and response shape from Amazon DynamoDB first (raw wire log, 83 request/response pairs), then validated dual-target.
tests/rustcovering each gap, including the per-item TransactWriteItemsCancellationReasonsshape for an invalid vector.EXTENDDB_EXPECT_VECTORS=1). The wire run exercises propagation delay 0; the delay>0 queue path is covered by 13 unit tests.cargo test --workspace,cargo fmt --check, andcargo clippy --all-targets -- -D warningsall pass.Checklist
cargo test --workspace)cargo fmt --check)cargo clippy -- -W clippy::pedantic)Storagetrait, auth model, on-disk format, or public CLI surface, an RFC has been accepted or is linked below. Otherwise, an ADR captures the decision (link below).ADR / RFC: docs/adr/0005-index-build-lifecycle-ownership.md (marked Accepted with an outcome section in this PR)
Breaking changes
n/a
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0 and I agree to the Developer Certificate of Origin (DCO). See CONTRIBUTING.md for details.