Skip to content

Commit 0e10091

Browse files
authored
fix(dynamodb): reset a removed per-GSI on-demand limit instead of silently no-oping (#1433)
1 parent cd92774 commit 0e10091

5 files changed

Lines changed: 216 additions & 3 deletions

File tree

docs/_generated/integ-last-run.tsv

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ drift-revert-arrays 2026-08-01T09:36:05Z PASS 110 verify.sh 0801 regression swee
7777
drift-revert-vpc 2026-07-30T16:18:56Z PASS 480 verify.sh post-rebase final for PR #1307 (#1299/#1300): 6/6 reverted, 21 del 0 err 0 orphans
7878
dsql 2026-07-30T18:11:24Z PASS 540 verify.sh 4-phase incl. destroy --remove-protection CC flip (#1312); orph clean
7979
dynamodb-autoscaling 2026-07-21T14:33:57Z PASS 77 verify.sh rc ok, orph clean
80-
dynamodb-globaltable 2026-08-09T13:35:05Z PASS 209 verify.sh issue #1387 GSI throughput asserted live; rc ok, orph clean
80+
dynamodb-globaltable 2026-08-09T14:33:56Z PASS 182 verify.sh #1423 PARTIAL removal asserted live (write reset absent, read kept 50); 3 del/0 err, 0 orphans
8181
dynamodb-gsi-update 2026-07-20T08:09:23Z PASS 579 verify.sh rc ok, orph clean
8282
dynamodb-ondemand 2026-08-01T10:07:21Z PASS 81 verify.sh 0801 regression sweep; ondemand+policy+kinesis backfills ok, 0 orphans
8383
dynamodb-sse 2026-08-01T10:07:21Z PASS 40 verify.sh 0801 regression sweep; SSE mapping ok, 0 orphans

src/provisioning/providers/dynamodb-globaltable-provider.ts

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1298,7 +1298,46 @@ export class DynamoDBGlobalTableProvider implements ResourceProvider {
12981298
// a flip to on-demand — the same silent-drop class #1387 exists to close.
12991299
const billingFlipped = oldBilling !== newBilling;
13001300
const update: UpdateGlobalSecondaryIndexAction = { IndexName: gsi.IndexName };
1301-
if (gsi.OnDemandThroughput) update.OnDemandThroughput = gsi.OnDemandThroughput;
1301+
// On-demand limits: MERGE the desired side with an explicit reset for
1302+
// every member the template DROPPED. Omitting a removed member leaves
1303+
// the old ceiling live in AWS forever while cdkd reports success — the
1304+
// absent-field-reset silent-drop class (#1160), one level down inside a
1305+
// nested block (#1225). CloudFormation resets it.
1306+
//
1307+
// Merging rather than branching is load-bearing: the read limit comes
1308+
// from `Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughput-
1309+
// Settings` and the write limit from `GSI.WriteOnDemandThroughputSettings`
1310+
// — two INDEPENDENT CDK props. An `else if` that only fired when the new
1311+
// side had no on-demand block at all would still silently drop the
1312+
// single-member removal, which is the likelier user edit.
1313+
//
1314+
// `-1` is the reset sentinel, LIVE-VERIFIED rather than inferred from
1315+
// the table-level field's docs (issue #1423). Both payload shapes were
1316+
// probed against real AWS: `{-1, -1}` cleared both members, and the
1317+
// MIXED `{MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1}` was
1318+
// accepted and read back as `{MaxReadRequestUnits: 50}` — the dropped
1319+
// member cleared, the kept one preserved. In both cases the reset reads
1320+
// back as ABSENCE, never as -1, so drift comparisons must expect that.
1321+
//
1322+
// Skipped on a billing flip, where "no on-demand fields" is just the
1323+
// translation of a PROVISIONED side rather than a template removal.
1324+
const previousOnDemand = previousSdkByName.get(gsi.IndexName)?.OnDemandThroughput;
1325+
const onDemand: OnDemandThroughput = { ...gsi.OnDemandThroughput };
1326+
if (!billingFlipped && previousOnDemand) {
1327+
if (
1328+
previousOnDemand.MaxReadRequestUnits !== undefined &&
1329+
onDemand.MaxReadRequestUnits === undefined
1330+
) {
1331+
onDemand.MaxReadRequestUnits = ON_DEMAND_LIMIT_RESET;
1332+
}
1333+
if (
1334+
previousOnDemand.MaxWriteRequestUnits !== undefined &&
1335+
onDemand.MaxWriteRequestUnits === undefined
1336+
) {
1337+
onDemand.MaxWriteRequestUnits = ON_DEMAND_LIMIT_RESET;
1338+
}
1339+
}
1340+
if (Object.keys(onDemand).length > 0) update.OnDemandThroughput = onDemand;
13021341
// Provisioned capacity on a flip is step 4's job, so only a real
13031342
// same-billing-mode edit sends it from here.
13041343
if (!billingFlipped && gsi.ProvisionedThroughput) {
@@ -2787,6 +2826,16 @@ function asRecord(value: unknown): Record<string, unknown> | undefined {
27872826
: undefined;
27882827
}
27892828

2829+
/**
2830+
* The sentinel DynamoDB accepts to CLEAR a per-GSI on-demand request-unit
2831+
* ceiling (issue #1423). Live-verified against real AWS: `UpdateTable` with
2832+
* `GlobalSecondaryIndexUpdates[].Update.OnDemandThroughput = {-1, -1}` is
2833+
* accepted, and the member reads back ABSENT from `DescribeTable` afterwards —
2834+
* so it is genuinely reset to unlimited rather than stored as -1. Any drift /
2835+
* read-back comparison must therefore expect ABSENCE, not this value.
2836+
*/
2837+
const ON_DEMAND_LIMIT_RESET = -1;
2838+
27902839
/** Coerce a CFn numeric (CFn is stringly-typed) to a finite number. */
27912840
function toFiniteNumber(value: unknown): number | undefined {
27922841
if (value === undefined || value === null || value === '') return undefined;

tests/integration/dynamodb-globaltable/lib/dynamodb-globaltable-stack.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,17 @@ export class DynamoDBGlobalTableStack extends cdk.Stack {
204204
partitionKey: { name: 'owner', type: ddb.AttributeType.STRING },
205205
// -> Replicas[local].GlobalSecondaryIndexes[].ReadOnDemandThroughputSettings
206206
maxReadRequestUnits: 50,
207+
// Issue #1423: REMOVING this from the template must RESET the live
208+
// ceiling, not silently no-op. `drop-gsi-ondemand-limits` drops ONLY
209+
// the write limit and keeps the read one, which is the harder case:
210+
// the two are independent CDK props, so a fix that only reset when
211+
// the whole on-demand block disappeared would still drop this edit.
212+
// verify.sh asserts the write member is ABSENT while read is still
213+
// 50 (the reset reads back as absence, never as -1).
207214
// -> GlobalSecondaryIndexes[].WriteOnDemandThroughputSettings
208-
maxWriteRequestUnits: 60,
215+
...(updateMode.includes('drop-gsi-ondemand-limits')
216+
? {}
217+
: { maxWriteRequestUnits: 60 }),
209218
},
210219
],
211220
removalPolicy: cdk.RemovalPolicy.DESTROY,

tests/integration/dynamodb-globaltable/verify.sh

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,34 @@ echo "[verify] step 13: cdkd deploy with CDKD_TEST_UPDATE=ttl,tags (structural t
407407
# cleans up the table regardless of TTL state.
408408
CDKD_TEST_UPDATE=ttl,tags ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose
409409

410+
echo "[verify] step 13b: cdkd deploy with drop-gsi-ondemand-limits (issue #1423 — REMOVING a per-GSI on-demand limit must RESET it, not no-op)"
411+
CDKD_TEST_UPDATE=ttl,tags,drop-gsi-ondemand-limits ${CLI} deploy "${STACK}" --state-bucket "${STATE_BUCKET}" --verbose
412+
413+
# The reset reads back as ABSENCE, never as -1 (live-probed on #1423). Pre-fix
414+
# the template removal emitted nothing at all, so the 60 write ceiling stayed
415+
# live in AWS forever while cdkd reported success.
416+
#
417+
# Assert the index still EXISTS first: a `| [0]` query against a MISSING index
418+
# also answers "None", so the absence check alone would pass if `byOwner` had
419+
# vanished entirely.
420+
OD_IDX="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \
421+
--query "length(Table.GlobalSecondaryIndexes[?IndexName=='byOwner'])" --output text)"
422+
if [ "${OD_IDX}" != "1" ]; then
423+
echo "FAIL: byOwner index missing after the drop-limits update (count=${OD_IDX})" >&2
424+
exit 1
425+
fi
426+
# The DROPPED member must be gone...
427+
OD_WRITE="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \
428+
--query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput.MaxWriteRequestUnits | [0]" --output text)"
429+
# ...while the one the template STILL declares must survive untouched.
430+
OD_READ="$(aws dynamodb describe-table --table-name "${GSI_OD_TABLE}" --region "${REGION}" \
431+
--query "Table.GlobalSecondaryIndexes[?IndexName=='byOwner'].OnDemandThroughput.MaxReadRequestUnits | [0]" --output text)"
432+
if [ "${OD_WRITE}" != "None" ] || [ "${OD_READ}" != "50" ]; then
433+
echo "FAIL: issue #1423 — expected write=None (reset) / read=50 (kept), got write=${OD_WRITE} / read=${OD_READ}" >&2
434+
exit 1
435+
fi
436+
echo " per-GSI write limit reset (absent) and read limit kept at 50, issue #1423 closed"
437+
410438
echo "[verify] step 14a: assert DeletionProtectionEnabled flipped back to false on AWS"
411439
DP_FINAL="$(aws dynamodb describe-table --table-name "${TABLE_NAME}" --region "${REGION}" \
412440
--query 'Table.DeletionProtectionEnabled' --output text)"

tests/unit/provisioning/dynamodb-globaltable-provider-gsi-throughput.test.ts

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,133 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => {
516516
});
517517

518518
describe('update()', () => {
519+
it('resets a REMOVED per-GSI on-demand limit with -1 instead of no-oping (issue #1423)', async () => {
520+
// Deleting `maxReadRequestUnits` / `maxWriteRequestUnits` from a template
521+
// used to emit nothing, so the old ceiling stayed live in AWS forever
522+
// while cdkd reported success — the absent-field-reset silent-drop class
523+
// (#1160). `-1` is the reset sentinel, LIVE-VERIFIED against real AWS:
524+
// UpdateTable accepted it on the per-GSI Update action and DescribeTable
525+
// then reported the member ABSENT.
526+
const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
527+
const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
528+
// Drop BOTH limits: write lives on the GSI, read on the local replica.
529+
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
530+
'WriteOnDemandThroughputSettings'
531+
];
532+
delete (
533+
(next['Replicas'] as Record<string, unknown>[])[0]!['GlobalSecondaryIndexes'] as Record<
534+
string,
535+
unknown
536+
>[]
537+
)[0]!['ReadOnDemandThroughputSettings'];
538+
539+
await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
540+
541+
const gsiUpdates = mockSend.mock.calls
542+
.map((c) => c[0])
543+
.filter(
544+
(c): c is UpdateTableCommand =>
545+
c instanceof UpdateTableCommand &&
546+
(c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined)
547+
);
548+
expect(gsiUpdates).toHaveLength(1);
549+
expect(gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates).toEqual([
550+
{
551+
Update: {
552+
IndexName: 'gsi2',
553+
OnDemandThroughput: { MaxReadRequestUnits: -1, MaxWriteRequestUnits: -1 },
554+
},
555+
},
556+
]);
557+
});
558+
559+
it('resets the DROPPED member while KEEPING the one still declared (partial removal)', async () => {
560+
// The likeliest user edit: read and write limits are two independent CDK
561+
// props (read via the local replica, write on the GSI), so removing ONE
562+
// is common. A branch that only reset when the new side had NO on-demand
563+
// block at all would silently leave the other ceiling live — the very
564+
// #1423 bug, shipped as fixed. Live-probed: the MIXED payload
565+
// {MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1} is accepted and
566+
// reads back as {MaxReadRequestUnits: 50}.
567+
const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
568+
const next = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
569+
// Drop ONLY the write limit; the replica-side read limit (50) stays.
570+
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
571+
'WriteOnDemandThroughputSettings'
572+
];
573+
574+
await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
575+
576+
const gsiUpdates = mockSend.mock.calls
577+
.map((c) => c[0])
578+
.filter(
579+
(c): c is UpdateTableCommand =>
580+
c instanceof UpdateTableCommand &&
581+
(c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined)
582+
);
583+
expect(gsiUpdates).toHaveLength(1);
584+
expect(
585+
gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.OnDemandThroughput
586+
).toEqual({ MaxReadRequestUnits: 50, MaxWriteRequestUnits: -1 });
587+
});
588+
589+
it('resets ONLY the member that was actually set before', async () => {
590+
// A blanket {-1, -1} would clear a sibling limit the template still
591+
// declares.
592+
const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
593+
// Previous side has WRITE only (drop the replica-side read limit).
594+
delete (
595+
(previous['Replicas'] as Record<string, unknown>[])[0]![
596+
'GlobalSecondaryIndexes'
597+
] as Record<string, unknown>[]
598+
)[0]!['ReadOnDemandThroughputSettings'];
599+
const next = structuredClone(previous) as Record<string, unknown>;
600+
delete (next['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
601+
'WriteOnDemandThroughputSettings'
602+
];
603+
604+
await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
605+
606+
const gsiUpdates = mockSend.mock.calls
607+
.map((c) => c[0])
608+
.filter(
609+
(c): c is UpdateTableCommand =>
610+
c instanceof UpdateTableCommand &&
611+
(c.input.GlobalSecondaryIndexUpdates ?? []).some((u) => u.Update !== undefined)
612+
);
613+
expect(gsiUpdates[0]!.input.GlobalSecondaryIndexUpdates?.[0]?.Update?.OnDemandThroughput).toEqual(
614+
{ MaxWriteRequestUnits: -1 }
615+
);
616+
});
617+
618+
it('does NOT emit a reset when the index simply had no limits before', async () => {
619+
const previous = structuredClone(ON_DEMAND_TABLE_PROPS) as Record<string, unknown>;
620+
delete (previous['GlobalSecondaryIndexes'] as Record<string, unknown>[])[0]![
621+
'WriteOnDemandThroughputSettings'
622+
];
623+
delete (
624+
(previous['Replicas'] as Record<string, unknown>[])[0]![
625+
'GlobalSecondaryIndexes'
626+
] as Record<string, unknown>[]
627+
)[0]!['ReadOnDemandThroughputSettings'];
628+
const next = structuredClone(previous) as Record<string, unknown>;
629+
// Unrelated edit so the table still goes through update().
630+
next['TableClass'] = 'STANDARD_INFREQUENT_ACCESS';
631+
632+
await provider.update('OnDemand', 'od-table', RESOURCE_TYPE, next, previous);
633+
634+
const resets = mockSend.mock.calls
635+
.map((c) => c[0])
636+
.filter(
637+
(c): c is UpdateTableCommand =>
638+
c instanceof UpdateTableCommand &&
639+
(c.input.GlobalSecondaryIndexUpdates ?? []).some(
640+
(u) => u.Update?.OnDemandThroughput !== undefined
641+
)
642+
);
643+
expect(resets).toHaveLength(0);
644+
});
645+
519646
it('carries ProvisionedThroughput on a GSI Create action (added index)', async () => {
520647

521648
const previous = { ...PROVISIONED_TABLE_PROPS, GlobalSecondaryIndexes: [] };

0 commit comments

Comments
 (0)