fix: report per-index ConsumedCapacity on transactional and PartiQL writes - #188
Merged
Merged
Conversation
TransactWriteItems reported a Table arm and nothing else under INDEXES, and sized that arm from the request payload rather than the item images. The transactional 2x factor applies to the table arm alone, so an index arm inside a transaction costs what it costs outside one.
ExecuteStatement and ExecuteTransaction join the same model, and BatchExecuteStatement gains a capacity surface it never had. Also adds the four request validations AWS enforces on batches and transactions, which turn some previously accepted requests into errors. A PartiQL SELECT against an index is untouched and still wrong (#179).
The image sizes a same-token replay bills against move onto the types the idempotency caches hold, so neither response type carries a pub(crate) field. BatchStatementRequest becomes non_exhaustive: it is two fields short of DynamoDB's, so it will gain fields again.
Four surfaces each sorted the aggregated tables and built one ConsumedCapacity per table, and three also folded per-table read units. Both now live beside the aggregation. The batch failure surcharge stays at its call site, being the one rule of the four that diverges.
Classification, duplicate detection and execution each parsed the whole statement list. Measured on a 25-statement batch: 244.8 to 166.7 us, a third of the call. Parsing was the larger half of that by more than two to one, against everything target resolution does, which inverts the guess that the per-statement metadata load dominated. It does not, on native, where table metadata is already cached in memory; on the wasm backend, which has no such cache, the balance is expected to fall the other way. The prepared item carries the resolved target beside the parsed statement, so duplicate detection reuses it rather than resolving again. Parse failure stays a per-statement error rather than a request-level one, which a Result<Vec<_>, _> would have quietly changed. Adds the batch to both benchmark suites. The PartiQL surfaces had no coverage in either, so the blocking instruction-count gate could not see a change to this path at all.
The parse-once change recovers 81.0 us on a 25-statement batch. That is the number to trust: an earlier run on a different workload recovered 78.1, and two workloads agreeing within 4 per cent while differing in everything else is the signature of a change whose mechanism is two fewer parses and nothing else. Supersedes the figures in 27419e7, which are wrong. Its benchmark inserted a string into a sort key declared N, so all 25 statements were rejected; BatchExecuteStatement reports member failures inside a successful response, so the call returned Ok and nothing complained. Its 244.8 to 166.7 us, its 33 per cent, its predicted-versus-measured agreement and its parse-versus-resolution ratio all describe the rejection path. The corrected figures are 495.7 to 414.7, about 16 per cent, and against the merge base the batch path is 387.9 to 414.7, about 7 per cent slower, which is what the four validations cost. Both benchmarks now assert the workload succeeded before timing it, and the merge-gating instruction-count one no longer measures 25 rejections. Also restores the mode guard ahead of aggregation on both transactional surfaces, which the shared-helper extraction had moved inside the builder so that every write transaction cloned its charge set before discarding it; sorts the transactional read and replay paths by table name, which the write path already did; and drops a doc claim about sharing a key-schema lookup that was never implemented.
The compatibility summary said TransactWriteItems and PartiQL writes report no per-index breakdown. That was true when it was written and the code has since caught up.
Contributor
Criterion Benchmark ResultsBaseline is the per-benchmark median of the last 5 stored runs, so one unusually fast or slow runner cannot skew the comparison. The range column is the spread across those runs.
Runs in the baseline
|
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
1 participant
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.
Closes #178.
What this changes
TransactWriteItems,ExecuteStatement,ExecuteTransactionandBatchExecuteStatementreported aTablearm and nothing else underINDEXES.They now report the per-index breakdown the single-item paths already did, and the
index units fold into the total, so figures move under
TOTALas well on anindexed table.
The rule that decides this, and the one worth reviewing carefully, is that the
transactional 2x factor applies to the base table arm alone. An index arm inside a
transaction costs what the same write costs outside one. A GSI key move charges the
index the same either way while the table arm doubles.
Checking that against real DynamoDB turned up four more divergences in the same
paths, all fixed here:
DeleteandConditionCheckcarry a key and no item, so both were sized on thekey: deleting a 3KB item reported 2 units against DynamoDB's 6. A
ConditionCheckwrites nothing and is still charged on the image it read.
replayed delete of a 9KB item reported 2 against DynamoDB's 6. It is now charged
against the images the first call was sized on, at 4KB read granularity.
BatchExecuteStatementhad no way to report capacity at all. It now acceptsReturnConsumedCapacityand aggregates per table across the batch. A failedstatement is still charged the write it attempted, sized on the row already
stored rather than on the item the statement carried.
not mix reads with writes, and may not name the same item twice, reads included.
Both surfaces now reject both, top level, before any statement runs.
Everything above is captured against eu-west-2, 74 rows across the four surfaces.
Two things reviewers should know
This is behaviour-breaking twice over. A request that previously succeeded in a
mixed or duplicate shape now fails, and the reported numbers change. Both are
covered in
CHANGELOG.md.It costs about 7% on a 25-statement
BatchExecuteStatement, measured against958e340. That is the price of the four validations, and it is stated in thechangelog beside them rather than left for someone to find.
Checklist
cargo fmt --checkandcargo clippy -- -D warningspass locallyCHANGELOG.mdupdated if this is a user-visible change(MIT License and Apache License, Version 2.0)
DynamoDB compatibility note
This moves dynoxide towards DynamoDB on every point above, and each figure is
pinned to a captured value rather than to a derivation. One gap is deliberately
left: a PartiQL
SELECTagainst an index still scans the base table and drops itsWHEREclause, so its capacity lands on the wrong arm. That is a data-correctnessbug rather than a capacity one, it is tracked as #179, and fixing the capacity
figure before the qualifier is honoured would pin the wrong number.