fix(dynamodb): translate GlobalTable GSI throughput to the CreateTable SDK shape - #1422
Conversation
…e SDK shape The provider cast the CFn GlobalSecondaryIndexes blob raw to the SDK GlobalSecondaryIndex[], but the two schemas model per-GSI throughput completely differently and the SDK v3 serializer silently drops unknown members. A PROVISIONED-billing GlobalTable with a GSI therefore failed CreateTable outright, and every TableV2 per-GSI on-demand limit vanished. TableV2 is the recommended L2 since CDK 2.95, so this is a daily-pattern surface. The issue's mapping table was materially incomplete and re-deriving it from the authoritative schemas changed the fix twice. WriteProvisionedThroughputSettings has exactly one member, WriteCapacityAutoScalingSettings, because write capacity on a GlobalTable is always auto-scaled; CreateTable needs a concrete number, so the mapping takes SeedCapacity before MinCapacity. Per-GSI read capacity is not on the top-level GSI at all, but on the local replica's GSI entry, so the SDK's single throughput object has to be fused from both halves. Applied at create, addReplica, the update replica-modify action, the update GSI diff (both sides translated before diffing, so an auto-scaling-only edit no longer emits an empty Update action AWS rejects), and the PAY_PER_REQUEST to PROVISIONED billing flip, which requires per-index throughput in the same UpdateTable call. Replica resource policies and per-index contributor insights need separate APIs and are recorded in a JSDoc block rather than dropped silently. Closes #1387
…, and stop warning on a flip Two defects found by code review of the GSI throughput translation. The billing-flip call built its index updates from the NEW template but filtered them by the names in the PREVIOUS one, so an index the deploy removes was silently omitted. Its Delete is issued in step 6, which runs after the flip, so at flip time it is still a live index on a table becoming PROVISIONED and AWS rejects the call for having no capacity on it. Its throughput now comes from the previous template, and only surviving indexes are marked handled so the removed one still reaches its Delete. The modified-GSI loop also warned that the index changed in a way UpdateTable cannot express whenever the BillingMode flipped, because the two sides are translated under different billing modes and therefore always differ. That warning is false and sends the user hunting a template problem that does not exist; the loop now skips entirely when the billing mode changed, since step 4 already applied the throughput change atomically. Both are bound by mutation-probed unit tests. Three pre-existing gaps the review surfaced are filed as #1419, #1420 and #1421.
…ew GSI helpers The GSI throughput translation moved the GlobalSecondaryIndexes read into delegated helpers, so the critic now records a delegated evidence tag alongside the existing element-read. CI drift-checks this file.
a7d0d32 to
f5dbd37
Compare
…y GSI blob, and carry WarmThroughput on add Three defects found by the 3-axis review of the first cut. Suppressing the false immutable-field warning by skipping the modified loop on any BillingMode flip was too broad. The flip call carries per-GSI fields only in the PAY_PER_REQUEST to PROVISIONED direction, so the reverse flip silently dropped every per-GSI Max Read/Write RequestUnits. The loop now sends the fields the new billing mode needs and suppresses only the warning; WarmThroughput rides along during a flip when it actually changed, so a simultaneous edit is not swallowed and an unchanged value costs no extra round trip. A non-array GlobalSecondaryIndexes value, such as an unresolved intrinsic, collapsed to an empty list and would have created the table with zero indexes while reporting success. That is the silent-drop class this work exists to close, one level up, so it now throws. Absent stays legitimately empty. WarmThroughput was translated by create but not forwarded by the update add path, so the same template produced a different index depending on whether the GSI was in the first deploy or a later one. Each fix is mutation-proofed: reverting any one of them kills a specific test.
…ping WarmThroughput on a flip Follow-up to the delta re-review of the previous round. create() has no wrapping catch at the translation point, so the helper's plain Error escaped untyped into the deploy engine's retry loop; the call site now converts it to a ProvisioningError. The same call was behind a truthiness gate, so a falsy-but-present value such as null skipped the non-array guard entirely and would have deployed a zero-GSI table. It is now called unconditionally, which is safe because the helper returns an empty list for an absent value. Indexes handled by the PAY_PER_REQUEST to PROVISIONED flip are skipped by the modified loop, so a simultaneous WarmThroughput change vanished with no warning. It now rides the flip's own Update action when it differs. The differs-check is applied in every case rather than only during a flip, because warm throughput is increase-only on the AWS side and re-asserting the current value on an unrelated capacity edit is a needless risk. The hand-rolled stringify comparison was replaced with the module's existing deepEqual. Each fix is mutation-proofed: reverting any one of them kills a specific test.
3-axis review of this PR — 1 blocker, 4 minorsHanding these off: another session owns this branch, so this comment records BLOCKER — a
|
…Table comments, coerce explicit throughput numbers Addresses the remaining findings from the 3-axis review carried out by a separate session. The existing-index scan in the billing flip runs before the translation, so a non-array GlobalSecondaryIndexes hit .map on a plain object and died with a bare TypeError instead of the named error the translator raises a few lines later. It is now Array.isArray-guarded like every other new site. Two comments claimed UpdateTable accepts only one of BillingMode, ReplicaUpdates and GlobalSecondaryIndexUpdates per call. That contradicted the flip path, which deliberately sends BillingMode and GlobalSecondaryIndexUpdates together because AWS requires per-GSI capacity in the same call. Both now state the real constraint and its documented exception. An explicitly supplied, already-SDK-shaped throughput block was cast straight through, the one path skipping toFiniteNumber, so a stringly-typed CFn value would reach the SDK unnormalized while every derived value was coerced. The three such sites now go through a shared helper that also drops junk keys.
The guard shipped in the previous commit without a test. Removing it makes the failure surface as 'previousCfnIndexes.map is not a function', an opaque error naming nothing, instead of the translator's named message.
…WS reads as inherit The numeric coercion added in the previous commit returned an empty object when no member parsed, and the call sites assigned it. AWS documents an empty ProvisionedThroughputOverride / OnDemandThroughputOverride as inherit the source table's settings, so an unparseable explicit value silently changed replica behavior instead of failing loudly. On the GSI side an empty block also suppressed the derived fallback, letting a garbage explicit value beat a valid derived one. The helper now returns undefined when nothing survives coercion, and every call site treats that as no explicit block and falls through. Found by review; both directions are bound by tests. Also moves the toFiniteNumber doc comment back onto toFiniteNumber, drops two redundant casts, and rewords the UpdateTable comments so the flip reads as the one case where BillingMode and GlobalSecondaryIndexUpdates combine rather than as an exception to the ReplicaUpdates rule.
Dropping the redundant as-casts left ProvisionedThroughput and OnDemandThroughput imported but unreferenced. Caught by CI because the previous round ran vp check --fix but not vp run check, and only the latter is CI parity.
…nparseable one The previous round's all-or-nothing coercion was wrong in both directions, found by review. A partial explicit block replaced the derived block wholesale, so an explicit MaxReadRequestUnits silently suppressed a valid derived MaxWriteRequestUnits. Explicit members now merge over derived ones per member instead. A fully unparseable explicit block fell through to the derived branch, which defaults to 5 read and write units, so a table the template explicitly sized was quietly deployed at the default. A present member that will not coerce now throws and names the field, matching what this provider already does for a non-array GlobalSecondaryIndexes. Absent members still fall through, which is the only case that should. Both directions are mutation-proofed. The first probe for the merge case did not actually apply, so it was re-run against a verified mutation.
… as issue #1428 The coercion started as a nit (a stringly-typed CFn value skipping toFiniteNumber on the one explicit-block path) and produced a new defect in each of three designs, every one of which passed my own review and was caught independently. v1 assigned an empty coerced block, which AWS reads as inherit the source table's settings, turning a loud failure into a silent one. v2 returned undefined and fell through to the derived branch, silently deploying the default 5 read and write units for a table the template explicitly sized. v3 threw on an uncoercible member, but the same helper also runs against previousProperties from state, so a garbage value already in state made every later update throw including the one that would remove it, and the replica path threw after CreateTable had already run. The three underlying defects are real but need one coherent fix (coerce, merge per member, gate on billing mode) with strictness applied only to the desired side and validation before any mutating call. That is issue #1428, which records all three failed designs so the next attempt does not rediscover them. Reverting restores the behavior this PR started from for that edge case, which the CFn schema forbids and CDK never emits.
## [0.278.12](v0.278.11...v0.278.12) (2026-08-09) ### Bug Fixes * **dynamodb:** translate GlobalTable GSI throughput to the CreateTable SDK shape ([#1422](#1422)) ([73442c1](73442c1))
|
🎉 This PR is included in version 0.278.12 🎉 The release is available on: Your semantic-release bot 📦🚀 |
1 participant
Summary
DynamoDBGlobalTableProvidercast the CloudFormationGlobalSecondaryIndexesblob RAW to the SDK'sGlobalSecondaryIndex[]. The two schemas model per-GSI throughput completely differently, and the AWS SDK v3 serializer silently drops unknown members, so:CreateTableoutright (AWS requiresProvisionedThroughputon every GSI), andTableV2per-GSI on-demand limit (maxReadRequestUnits/maxWriteRequestUnits) vanished without a trace.TableV2is the recommended CDK L2 since 2.95 and synthesizesAWS::DynamoDB::GlobalTable, so this is a daily-pattern surface.The issue's mapping table was incomplete, and re-deriving it changed the fix
The shapes were re-derived from the live
cloudformation:DescribeTypeschema and cross-checked against a realcdk synthofTableV2under both billing modes. Two corrections mattered:WriteProvisionedThroughputSettingshas exactly one member,WriteCapacityAutoScalingSettings. There is no literalWriteCapacityUnits— write capacity on a GlobalTable is always auto-scaled.CreateTableneeds a concrete number, so the mapping takesSeedCapacity(the documented "initial provisioned capacity units") beforeMinCapacity.Replicas[?Region==<deploy region>].GlobalSecondaryIndexes[].Read{Provisioned,OnDemand}ThroughputSettings, so the SDK's singleProvisionedThroughput/OnDemandThroughputobject has to be fused from both halves. The GSI-level spellings the schema also permits are honored as a fallback for hand-authored templates.The repo's captured
AWS-DynamoDB-GlobalTable.jsonfixture stores top-level names only and could not have settled either point.Mapping
GSI.WriteProvisionedThroughputSettings.WriteCapacityAutoScalingSettings.{SeedCapacity ?? MinCapacity}ProvisionedThroughput.WriteCapacityUnitsReplicas[local].GSI[].ReadProvisionedThroughputSettings.ReadCapacityUnits(fallback: GSI-level)ProvisionedThroughput.ReadCapacityUnitsGSI.WriteOnDemandThroughputSettings.MaxWriteRequestUnitsOnDemandThroughput.MaxWriteRequestUnitsReplicas[local].GSI[].ReadOnDemandThroughputSettings.MaxReadRequestUnitsOnDemandThroughput.MaxReadRequestUnitsGSI.WarmThroughputWarmThroughput(same spelling)Replicas[].GSI[].ReadProvisionedThroughputSettings.ReadCapacityUnitsProvisionedThroughputOverride.ReadCapacityUnitsReplicas[].GSI[].ReadOnDemandThroughputSettings.MaxReadRequestUnitsOnDemandThroughputOverride.MaxReadRequestUnitsCall sites
create()—CreateTable.GlobalSecondaryIndexestranslated instead of cast.addReplica()— replica GSI overrides translated.update()replica-modify — same translation onUpdateReplicationGroupMemberAction.update()GSI diff — both sides translated before diffing, so emittedCreate/Updateactions carry real throughput. Side benefit: an auto-scaling-only edit (MaxCapacity20 to 30 — invisible to the DynamoDB API) no longer produces a bareUpdate: { IndexName }that AWS rejects as empty; a modified GSI yielding no throughput at all now warns and skips.update()billing-mode flip — not named in the issue. AWS requires per-indexProvisionedThroughputin the sameUpdateTablecall that flipsPAY_PER_REQUESTtoPROVISIONED; without this the fix would have made create work while leaving the flip broken.Deliberately left unmapped (recorded in-code, not silently dropped)
This provider has no
unhandledByDesignmap, so the rationale lives in a JSDoc block onaddReplica:Replicas[].ReplicaStreamSpecification.ResourcePolicyandReplicas[].ResourcePolicy— both needPutResourcePolicy, not anyUpdateTablefield.Replicas[].GlobalSecondaryIndexes[].ContributorInsightsSpecification— needs a per-indexUpdateContributorInsights.handledPropertiesis unchanged, so no coverage regeneration was required.Review findings fixed in this PR
A 2-axis review (code quality + test adequacy, both mutation-tested rather than read-off) found two real defects in the new code, fixed here:
Deleteis issued in step 6, which runs AFTER the flip, so at flip time it is still a live index on a table becoming PROVISIONED — and by this PR's own premise AWS rejects the call for having no capacity on it. Its throughput now comes from the previous template, and only SURVIVING indexes are marked handled so the removed one still reaches itsDelete.PROVISIONED -> PAY_PER_REQUESTflip. The two sides are translated under different billing modes, so every GSI necessarily lands inmodifiedwith nothing to send and hit the "KeySchema / Projection are immutable" warning. That message is untrue and sends the user hunting a non-existent template problem; the loop now skips entirely when the billing mode changed, since step 4 already applied the change atomically.Three test-adequacy gaps were also closed: a test asserting members are OMITTED when unset (relaxing that guard previously passed the whole suite), a binding test for the warn-and-skip guard, and an honest comment on the auto-scaling-only test explaining that it is a both-must-hold end-state assertion rather than a binding test for either mechanism.
Deliberately not fixed here (filed)
WriteCapacityAutoScalingSettingsnow yields the correct INITIAL capacity, but nodynamodb:index:*scalable target is created, so the index never scales.create()registers no auto-scaling at all, even table-level; the two belong together.readCurrentStatestores the raw SDK GSI descriptions in the CFn-shaped state key, socdkd driftreports permanent phantom drift on any GSI-carrying GlobalTable. Pre-existing; a correct reverse map needs the Application Auto Scaling read that (AWS::DynamoDB::GlobalTable: per-GSI auto-scaling never registered (and create() registers no auto-scaling at all) #1419) is about, so half-fixing it would trade one phantom drift for another.AWS::DynamoDB::GlobalTableis still absent fromNESTED_KEY_TARGETS, which is exactly this bug class; critic target expansion is tracked in (audit: nested-key critic blind spots found by the 0809 sweep — mixed-case SDK models, file-global literal heuristic, missing targets, selective sub-key forwards #1393).Test plan
26 new unit tests in
tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts. Both property bags are copied verbatim from a realcdk synthrather than hand-invented — which is exactly what surfaced the read-capacity-lives-on-the-replica asymmetry a hand-written fixture would have encoded wrongly.Mutation-proofed, per call site. Reverting each of the five call sites individually kills a distinct test; so does removing the removed-index handling, the billing-flip dedupe, the warn-and-skip guard, and the omit-when-unset guard. Every assertion added by the review was verified to go red under its own mutation before being kept.
Real AWS integ (
/run-integ dynamodb-globaltable, us-east-1, 207s, PASS) — re-run after the review fixes, since those touched the provider: the fixture gains two unconditionalTableV2s (not gated behindCDKD_TEST_UPDATE, so the baseline deploy exercises the previously-failing create path) — two tables because the billing modes cannot coexist on one. Live assertions:byStatus.ProvisionedThroughput.ReadCapacityUnits = 7byStatus.ProvisionedThroughput.WriteCapacityUnits = 3byOwner.OnDemandThroughput.MaxReadRequestUnits = 50byOwner.OnDemandThroughput.MaxWriteRequestUnits = 60Destroy: 3 deleted, 0 errors, 0 orphans; state cleared.
verify.shstep 3 stopped taking "the firstAWS::DynamoDB::GlobalTablein state" (which with three tables would have grabbed an arbitrary one) in favor of selecting by logical-id prefix.Full suite: 520 files / 8868 tests pass.
Closes #1387