Skip to content

fix: Report per-index consumed capacity for writes (excludes transact/batch APIs for now)#199

Open
robinnsc wants to merge 2 commits into
mainfrom
fix/indexes-consumed-capacity-writes
Open

fix: Report per-index consumed capacity for writes (excludes transact/batch APIs for now)#199
robinnsc wants to merge 2 commits into
mainfrom
fix/indexes-consumed-capacity-writes

Conversation

@robinnsc

@robinnsc robinnsc commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

What

Implementing index consumed capacity accounting for the single item write operations: PutItem, UpdateItem, DeleteItem

Deferred Changes

Support for per index breakdowns for TransactWriteItems and BatchWriteItem not included, as it requires some material further changes in the storage layer with DataEngine traits, will be addressed in a followup. Also, exact byte sizing would still be a broader issue, and will resolved its own item size formula gap fix

Why

Requests with the ReturnConsumedCapacity parameter set to 'INDEXES' on a table with secondary indexes was returning only the base table number, with no per-index breakdown. TOTAL mode also under counted, reporting only the base-table units.

ConsumedCapacity was previously hardcoded as none for GSIs/LSIs, so the breakdown was never populated and the aggregate never included index writes.

Testing done

  • New consumed capacity index python tests added and verified against local ExtendDDB Postgres server and server side DDB
  • Rust tests padding
  • Existing consumed capacity tests all passing as expected
  • Previously failing index consumed capacity server side DDB functional tests now passing

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, 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).

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.

@robinnsc robinnsc changed the title fix: Report per-index consumed capacity for writes (excludes transact… fix: Report per-index consumed capacity for writes (excludes transact/batch APIs for now) Jul 2, 2026
@robinnsc robinnsc marked this pull request as ready for review July 2, 2026 19:29
@pdf-amzn

pdf-amzn commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Nice fix — the per-index capacity accounting is correct and the tests are solid. One architectural concern and a few minor items:

Main concern: redundant describe_table per write

The engine already fetches TableKeyInfo on every operation (via a cached lookup). The new code calls describe_table again to get the GSI/LSI definitions needed for per-index capacity. This is an extra catalog round-trip that could be avoided by enriching TableKeyInfo to carry index metadata:

pub struct IndexInfo {
    pub index_name: String,
    pub key_schema: Vec<KeySchemaElement>,
    pub projection: Projection,
}

pub struct TableKeyInfo {
    // ... existing fields ...
    pub global_secondary_indexes: Vec<IndexInfo>,
    pub local_secondary_indexes: Vec<IndexInfo>,
}

Since TableKeyInfo is already cached (CachedTableKeyInfoStore) and fetched on every operation, this adds zero extra I/O. The capacity_helpers::write_capacity_indexed function would take &TableKeyInfo instead of &TableDescription, and the describe_table calls in put/delete/update disappear. This also naturally sets up the transact/batch follow-up — those handlers already have TableKeyInfo per table via the lookup cache.

The current approach is functionally correct, but we do care enough about performance that I think we shoiudl do something here; enriching TableKeyInfo is one way to do that. Others might have other suggestions on the best way to do this.

Minor items (non-blocking):

  1. UpdateItem charges only the new item's index membership. DynamoDB charges both old and new projections — if an update removes a GSI key attribute, the old index entry is deleted (1 WCU charged to that GSI) even though the new item doesn't project into it. Edge case, fine as a follow-up.

  2. Test coverage uses only ALL projection. The project_index_item function handles KEYS_ONLY and INCLUDE, but no test exercises them. A test with a KEYS_ONLY GSI where the item has many non-key attributes would verify the per-index CU reflects the projected (smaller) size rather than the full item size.

  3. No LSI test. Code path is symmetric with GSI, but a single LSI test case would be cheap insurance.

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

Can we rebase this on #128 as the index logic may differ a lot once it lands?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants