Skip to content

fix(dynamodb): reset a removed per-GSI on-demand limit instead of silently no-oping - #1433

Merged
go-to-k merged 2 commits into
mainfrom
fix/1423-globaltable-ondemand-reset
Aug 9, 2026
Merged

fix(dynamodb): reset a removed per-GSI on-demand limit instead of silently no-oping#1433
go-to-k merged 2 commits into
mainfrom
fix/1423-globaltable-ondemand-reset

Conversation

@go-to-k

@go-to-k go-to-k commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Closes #1423

Summary

Removing maxReadRequestUnits / maxWriteRequestUnits from a
AWS::DynamoDB::GlobalTable GSI emitted nothing at all, so the old ceiling
stayed live in AWS forever while cdkd reported success. CloudFormation resets
it. This is the absent-field-reset silent-drop class (#1160), one level down
inside a nested block (#1225).

The blocking unknown, settled by a live probe

AWS documents -1 as "reset to default" for the table-level
OnDemandThroughput, and says nothing about the per-GSI
GlobalSecondaryIndexUpdates[].Update.OnDemandThroughput. Reset semantics are
field-specific, so this was probed rather than inferred (throwaway table,
deleted afterwards, no orphans):

1. CreateTable (PAY_PER_REQUEST) with per-GSI limits 50/60
   after create: {"MaxReadRequestUnits":50,"MaxWriteRequestUnits":60}
2. UpdateTable: per-GSI OnDemandThroughput = {-1, -1}
   UpdateTable ACCEPTED -1
   after -1 update: null          <- member ABSENT = reset to unlimited
3. DeleteTable -> deleted

So -1 is accepted and the limit is genuinely cleared — it reads back as
absence, never as -1. The full transcript is recorded on the issue for
anyone auditing the choice later.

Review fix-back (blocker)

The first version put the reset in an else if, so it only fired when the new
side had no OnDemandThroughput at all. The read limit comes from
Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings and
the write limit from GSI.WriteOnDemandThroughputSettings — two independent
CDK props — so removing just one left the other live in AWS forever. That is the
#1423 bug itself, in the likelier user edit, shipped as fixed. My unit test
missed it because its previous only had one member set.

Now it merges: start from the desired side, fill -1 for every member the
template dropped. The MIXED payload was live-probed before adopting it, since
only {-1, -1} had been verified:

2. UpdateTable: MIXED per-GSI OnDemandThroughput = keep 50 / reset -1
   UpdateTable ACCEPTED -1
   after -1 update: {"MaxReadRequestUnits":50}

Dropped member cleared, kept member preserved.

Implementation notes

  • Only the members that were actually SET before are reset. A blanket
    {-1, -1} would clear a sibling limit the template still declares.
  • Skipped on a billing flip, where "no on-demand fields" is just the
    translation of a PROVISIONED side, not a template removal.
  • The sentinel is a named constant whose JSDoc records both the live
    verification and the read-back-is-absence caveat, so a future drift /
    read-back comparison does not expect -1.

Test plan

Four unit tests: the full reset, the partial removal (keep read, reset
write) the review caught, the reset-only-what-was-set case, and the negative
that must NOT emit a reset when the index simply never had limits.

Real AWS: dynamodb-globaltable gained a drop-gsi-ondemand-limits update
phase that drops only the write limit — the harder case — and verify.sh
asserts the write member is absent while the read one is still 50. It also
asserts the index EXISTS first: a | [0] query against a missing index also
answers None, so the absence check alone would pass if byOwner had vanished.

[verify] step 13b: cdkd deploy with drop-gsi-ondemand-limits (issue #1423 ...)
    per-GSI write limit reset (absent) and read limit kept at 50, issue #1423 closed
Stack CdkdDynamoDBGlobalTableExample destroyed (3 deleted, 0 errors)
[verify] PASS

The first run of that assertion FAILED: the rewritten queries had lost the
Table. prefix so length() received null. Caught by the integ, fixed, re-run
clean — 182s, 3 deleted / 0 errors / 0 orphans. Full local gate: typecheck, lint,
build, 523 files / 8966 tests, vp run gen:all-matrices clean.

Not in scope

Filed as #1434: the table-level on-demand ceiling and the cross-region
replica OnDemandThroughputOverride have the SAME absent-field-reset class.
Each needs its own live probe (reset semantics are field-specific) and the
table-level one touches a different code path (flatUpdate / flatChanged)
than the per-GSI loop this PR reworked.

Also still open on #1423 itself: the non-array GlobalSecondaryIndexes inconsistency also noted on #1423
(toSdkGlobalSecondaryIndexes returns [] and create() drops it, while the
per-ENTRY policy two lines down deliberately passes a malformed entry through
so AWS reports the real error). Changing it means sending a known-bad shape to
AWS — a behavior change that wants its own test rather than a rider on a reset
fix. #1423 stays open for it.

…ently no-oping

Closes #1423

Removing maxReadRequestUnits / maxWriteRequestUnits from a GlobalTable GSI emitted nothing, so the old ceiling stayed live in AWS forever while cdkd reported success. CloudFormation resets it. This is the absent-field-reset silent-drop class (#1160) one level down inside a nested block (#1225).

The blocking unknown was whether -1 is the reset sentinel for the per-GSI Update action: AWS documents it for the TABLE-level OnDemandThroughput only, and reset semantics are field-specific. Settled by a live probe rather than inferred - a real UpdateTable with {-1, -1} on GlobalSecondaryIndexUpdates[].Update was ACCEPTED and DescribeTable then reported the member ABSENT, i.e. genuinely cleared rather than stored as -1. The probe transcript is recorded on the issue.

Only the members that were actually SET before are reset; a blanket {-1, -1} would clear a sibling limit the template still declares. Skipped on a billing flip, where absent on-demand fields are just the translation of a PROVISIONED side rather than a template removal.

3 unit tests (2 fail against origin/main; the third is the negative that must NOT emit a reset) plus a new drop-gsi-ondemand-limits integ phase asserting the live DescribeTable readback is ABSENT. Integ PASS, 3 deleted / 0 errors / 0 orphans.
…still dropped

Review blocker on the first version: the reset lived in an else-if, so it only fired when the new side had NO OnDemandThroughput at all. The read limit comes from Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings and the write limit from GSI.WriteOnDemandThroughputSettings - two INDEPENDENT CDK props - so removing just one left the other live in AWS forever. That is the #1423 bug itself, in the likelier user edit, shipped as fixed.

Now merges: start from the desired side and fill -1 for every member the template dropped. Live-probed the MIXED payload before adopting it, since only {-1,-1} had been verified: UpdateTable accepted { MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1 } and DescribeTable read back { MaxReadRequestUnits: 50 } - dropped member cleared, kept member preserved.

The integ fixture now drops ONLY the write limit (the harder case) and verify.sh asserts write is absent while read is still 50. It also asserts the index EXISTS first: a '| [0]' query against a missing index also answers None, so the absence check alone would pass if byOwner had vanished.

The first integ run of that assertion FAILED because the rewritten queries lost the Table. prefix and length() received null - caught by the integ, fixed, re-run clean (182s, 3 deleted / 0 errors / 0 orphans).

Table-level and cross-region-replica on-demand ceilings have the same class and are filed as #1434; each needs its own live probe and the table-level one touches a different code path.
@go-to-k
go-to-k merged commit 0e10091 into main Aug 9, 2026
6 checks passed
@go-to-k
go-to-k deleted the fix/1423-globaltable-ondemand-reset branch August 9, 2026 14:39
github-actions Bot pushed a commit that referenced this pull request Aug 9, 2026
## [0.278.15](v0.278.14...v0.278.15) (2026-08-09)

### Bug Fixes

* **dynamodb:** reset a removed per-GSI on-demand limit instead of silently no-oping ([#1433](#1433)) ([0e10091](0e10091))
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

🎉 This PR is included in version 0.278.15 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AWS::DynamoDB::GlobalTable: removing a per-GSI on-demand limit silently no-ops (absent-field reset, #1160 class)

1 participant