fix(dynamodb): address 3-axis review of the GlobalTable auto-scaling fix

Blocker: step 6b confined itself to the two never-registered dimensions
via a STATIC filter, handing table-level write and cross-region read
back to their existing diff gates. Those gates are exactly the ones
the backfill argument says never fire on a pre-fix table (an unchanged
replica is not even visited by diffReplicas), so the static filter
permanently excluded two dimensions from the fix it was part of. 6b now
covers all four; double-application is avoided with a DYNAMIC skip-set
of what earlier steps applied during THIS update, so a dimension whose
gate declined is still backfilled.

Cost: re-asserting every target on every deploy is
2 x (1 + N_gsi x (1 + N_replica)) serial calls -- over a hundred round
trips on a 20-GSI, 3-replica table. Presence is now probed with one
batched DescribeScalableTargets per region and an already-present,
unchanged target is skipped. A failed probe means presence is unknown
and everything is upserted, which is the correct direction to fail.

Silent gap under load: every error in this path is swallowed into a
WARN, so an un-retried ThrottlingException would leave a target
unregistered without a trace -- the same never-registered gap, recreated
by the burst this change introduces. RegisterScalableTarget and
PutScalingPolicy now carry a throttle-only retry.

Create-side leak: the partial-create cleanup deletes the table directly
rather than routing through delete(), so a target registered before a
LATER wiring step failed was orphaned with no table left to name it.
Registration is now the last wiring step and is wrapped so a
best-effort concern can never destroy a successfully created table.

Cross-region teardown leak: index names came from the replica's own
GlobalSecondaryIndexes, which AWS may omit for a replica that inherits
throughput (ProvisionedThroughputOverride is documented "if not
described, uses the source table's"). Index names are identical across
replicas, so the table's list is the correct source.

Index readiness: a GSI added by the same deploy leaves the TABLE ACTIVE
while the index is still CREATING, and application-autoscaling rejects
a target whose resource is not ready. Step 6b waits for index
readiness first -- best-effort, since a miss self-heals on the next
deploy but a throw would fail a deploy whose resources are correct.

Also: dropped a dangling duplicate JSDoc block and corrected two
comments that still claimed SeedCapacity-before-MinCapacity.

Tests: +6. The identical-on-both-sides roundtrip test encoded the OLD
contract (unchanged template means no calls) and is replaced by two
tests pinning the new one -- no-op when registered, BACKFILL when not.
New coverage for the create-side ordering, the create-side guard, the
cross-region index teardown, ObjectNotFound suppression, and the
table-level seed context (which no fixture in the tree pinned).

Integ: the fixture GSI's read capacity becomes autoscaled so the fourth
dimension gets real-AWS coverage (minCapacity 7 keeps step 4b's
existing assertion intact), the table-level write gains a seedCapacity
differing from its min so #1435 is discriminating at table level too,
and step 16a2 asserts all four dimensions are deregistered rather than
just the index write one.

Author: go-to-k

docs/changelog-cdkd.md

@@ -165,9 +165,16 @@ export class DynamoDBGlobalTableStack extends cdk.Stack {
         readCapacity: ddb.Capacity.fixed(5),
         // TableV2 requires auto-scaled write capacity (the GlobalTable CFn
         // shape has no literal WriteCapacityUnits).
+        //
+        // `seedCapacity` (8) differs from `minCapacity` (1) so the TABLE-level
+        // half of issue #1435 is discriminating against real AWS: verify.sh
+        // asserts the created table-level WriteCapacityUnits is 1. Without a
+        // seed here, flipping the table-level call site back to 'seed' would
+        // break no assertion and only the per-index value would be pinned.
         writeCapacity: ddb.Capacity.autoscaled({
           minCapacity: 1,
           maxCapacity: 10,
+          seedCapacity: 8,
           targetUtilizationPercent: 70,
         }),
       }),
@@ -176,7 +183,18 @@ export class DynamoDBGlobalTableStack extends cdk.Stack {
           indexName: 'byStatus',
           partitionKey: { name: 'status', type: ddb.AttributeType.STRING },
           // -> Replicas[local].GlobalSecondaryIndexes[].ReadProvisionedThroughputSettings
-          readCapacity: ddb.Capacity.fixed(7),
+          //
+          // AUTOSCALED rather than fixed so the fourth scalable dimension,
+          // `dynamodb:index:ReadCapacityUnits`, gets real-AWS coverage — with
+          // a fixed read capacity that dimension is never registered and half
+          // the new index-level surface would be unit-tested only. minCapacity
+          // is 7 so step 4b's existing `ReadCapacityUnits = 7` assertion holds
+          // unchanged (issue #1435: a create takes MinCapacity).
+          readCapacity: ddb.Capacity.autoscaled({
+            minCapacity: 7,
+            maxCapacity: 70,
+            targetUtilizationPercent: 65,
+          }),
           // -> GlobalSecondaryIndexes[].WriteProvisionedThroughputSettings
           //    .WriteCapacityAutoScalingSettings.
           //

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

@@ -286,6 +286,32 @@ case "${INDEX_POLICY_TARGET}" in
     ;;
 esac
 
+# The READ half of the same index. Registered from a DIFFERENT CFn location
+# than the write half (`Replicas[local].GlobalSecondaryIndexes[]` rather than
+# the top-level GSI), so it is a genuinely separate path, not a mirror.
+INDEX_READ_MINMAX="$(aws application-autoscaling describe-scalable-targets \
+  --service-namespace dynamodb \
+  --resource-ids "${INDEX_RESOURCE_ID}" \
+  --scalable-dimension dynamodb:index:ReadCapacityUnits \
+  --region "${REGION}" \
+  --query 'ScalableTargets[0].[MinCapacity,MaxCapacity]' --output text)"
+if [ "${INDEX_READ_MINMAX}" != "7	70" ]; then
+  echo "[verify] FAIL (issue #1419): no dynamodb:index:ReadCapacityUnits target on ${INDEX_RESOURCE_ID}" >&2
+  echo "[verify]   got Min/Max '${INDEX_READ_MINMAX}', expected tab-separated '7' and '70'" >&2
+  exit 1
+fi
+echo "[verify] step 4c ok: index read autoscaling registered (${INDEX_READ_MINMAX})"
+
+# Issue #1435 at TABLE level: the fixture declares minCapacity 1 / seedCapacity 8,
+# and CloudFormation creates at MinCapacity. Pre-fix cdkd sent the seed (8).
+PROV_TABLE_WRITE="$(aws dynamodb describe-table --table-name "${GSI_PROV_TABLE}" --region "${REGION}" \
+  --query 'Table.ProvisionedThroughput.WriteCapacityUnits' --output text)"
+if [ "${PROV_TABLE_WRITE}" != "1" ]; then
+  echo "[verify] FAIL (issue #1435): ${GSI_PROV_TABLE} table-level WriteCapacityUnits is '${PROV_TABLE_WRITE}' (expected 1 = MinCapacity, not 8 = SeedCapacity)" >&2
+  exit 1
+fi
+echo "[verify] step 4c ok: table-level write capacity created at MinCapacity (${PROV_TABLE_WRITE})"
+
 echo "[verify] step 5 (was steps 5/6/7): cdkd deploy with CDKD_TEST_UPDATE=deletion-protection (in-place update — Issue #389)"
 # ORDER NOTE (PR follow-up to #403): TTL toggle is intentionally
 # deferred to the END of the integ flow. AWS's DynamoDB
@@ -587,17 +613,27 @@ echo "[verify] step 16a2 (Issue #1419): assert the per-INDEX scalable target did
 # remove a registered target. An orphan `table/<t>/index/<i>` target is
 # silently inherited by a future table of the same name, so delete() has to
 # deregister the index dimensions the way it already did the table ones.
-INDEX_TARGETS_AFTER="$(aws application-autoscaling describe-scalable-targets \
-  --service-namespace dynamodb \
-  --resource-ids "table/${GSI_PROV_TABLE}/index/byStatus" \
-  --scalable-dimension dynamodb:index:WriteCapacityUnits \
-  --region "${REGION}" \
-  --query 'length(ScalableTargets)' --output text)"
-if [ "${INDEX_TARGETS_AFTER}" != "0" ]; then
-  echo "[verify] FAIL (issue #1419): index scalable target survived destroy (count=${INDEX_TARGETS_AFTER}, expected 0)" >&2
-  exit 1
-fi
-echo "[verify] step 16a2 ok: index scalable target deregistered on destroy"
+assert_target_gone() { # $1 = resource id, $2 = scalable dimension
+  local remaining
+  remaining="$(aws application-autoscaling describe-scalable-targets \
+    --service-namespace dynamodb \
+    --resource-ids "$1" \
+    --scalable-dimension "$2" \
+    --region "${REGION}" \
+    --query 'length(ScalableTargets)' --output text)" || return 1
+  if [ "${remaining}" != "0" ]; then
+    echo "[verify] FAIL (issue #1419): scalable target $1 ($2) survived destroy (count=${remaining}, expected 0)" >&2
+    exit 1
+  fi
+  echo "[verify] step 16a2 ok: $2 on $1 deregistered"
+}
+# BOTH index dimensions, and the local table read dimension this change is
+# what first registers. A teardown that covers only the write half leaks the
+# other three onto whatever table next takes this name.
+assert_target_gone "table/${GSI_PROV_TABLE}/index/byStatus" dynamodb:index:WriteCapacityUnits
+assert_target_gone "table/${GSI_PROV_TABLE}/index/byStatus" dynamodb:index:ReadCapacityUnits
+assert_target_gone "table/${TABLE_NAME}" dynamodb:table:ReadCapacityUnits
+assert_target_gone "table/${TABLE_NAME}" dynamodb:table:WriteCapacityUnits
 
 echo "[verify] step 16b: assert cdkd state is empty"
 assert_gone "cdkd state file still exists at s3://${STATE_BUCKET}/${STATE_KEY}" aws s3api head-object --bucket "${STATE_BUCKET}" --key "${STATE_KEY}"

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

@@ -76,6 +76,7 @@ vi.mock('../../../src/utils/logger.js', () => {
 
 import {
   DynamoDBGlobalTableProvider,
+  derivePerCallProvisionedThroughput,
   deriveReadCapacityUnits,
   deriveWriteCapacityUnits,
   toSdkGlobalSecondaryIndexes,
@@ -260,6 +261,34 @@ describe('DynamoDBGlobalTable GSI throughput translation (issue #1387)', () => {
       ).toBe(2);
     });
 
+    it('applies the source to the TABLE-level write block, not just per-GSI', () => {
+      // The table-level flip call site passes 'seed' too. Without this, every
+      // fixture in the tree carries SeedCapacity only on a GSI, so flipping
+      // that call site to 'min' would break nothing and the seed context
+      // would be pinned per-index only.
+      const props = {
+        WriteProvisionedThroughputSettings: {
+          WriteCapacityAutoScalingSettings: { MinCapacity: 4, MaxCapacity: 40, SeedCapacity: 31 },
+        },
+        Replicas: [
+          {
+            Region: 'us-east-1',
+            ReadProvisionedThroughputSettings: {
+              ReadCapacityAutoScalingSettings: { MinCapacity: 6, MaxCapacity: 60, SeedCapacity: 22 },
+            },
+          },
+        ],
+      };
+      expect(derivePerCallProvisionedThroughput(props, 'us-east-1')).toEqual({
+        ReadCapacityUnits: 6,
+        WriteCapacityUnits: 4,
+      });
+      expect(derivePerCallProvisionedThroughput(props, 'us-east-1', 'seed')).toEqual({
+        ReadCapacityUnits: 22,
+        WriteCapacityUnits: 31,
+      });
+    });
+
     it('takes MinCapacity over SeedCapacity on the read side too', () => {
       expect(
         deriveReadCapacityUnits({

tests/integration/dynamodb-globaltable/verify.sh

@@ -1,5 +1,10 @@
 import { describe, it, expect, vi, beforeEach } from 'vite-plus/test';
-import { DescribeTableCommand, ResourceNotFoundException } from '@aws-sdk/client-dynamodb';
+import {
+  DescribeTableCommand,
+  DeleteTableCommand,
+  UpdateTimeToLiveCommand,
+  ResourceNotFoundException,
+} from '@aws-sdk/client-dynamodb';
 import {
   RegisterScalableTargetCommand,
   PutScalingPolicyCommand,
@@ -327,6 +332,64 @@ describe('DynamoDBGlobalTable per-index auto-scaling (issue #1419)', () => {
       });
     });
 
+    it('registers LAST, so a failed wiring step can never orphan a target it created', async () => {
+      // The partial-create cleanup deletes the table directly (it does not
+      // route through delete(), so it deregisters nothing). A target
+      // registered before a later wiring step failed would therefore be
+      // orphaned in the application-autoscaling control plane with no table
+      // left to name it. Registration is deliberately the last wiring step;
+      // this pins it by failing the step that used to run after it (TTL).
+      mockSend.mockImplementation((command: unknown) => {
+        if (command instanceof UpdateTimeToLiveCommand) {
+          return Promise.reject(new Error('ttl boom'));
+        }
+        if (command instanceof DescribeTableCommand) {
+          return Promise.resolve({
+            Table: {
+              TableName: TABLE_NAME,
+              TableArn: TABLE_ARN,
+              TableStatus: 'ACTIVE',
+              GlobalSecondaryIndexes: [{ IndexName: 'gsi1', IndexStatus: 'ACTIVE' }],
+              Replicas: [{ RegionName: 'us-east-1', ReplicaStatus: 'ACTIVE' }],
+            },
+          });
+        }
+        return Promise.resolve({});
+      });
+
+      await expect(
+        provider.create('Prov', RESOURCE_TYPE, {
+          ...AUTOSCALED_PROPS,
+          TimeToLiveSpecification: { AttributeName: 'expiresAt', Enabled: true },
+        })
+      ).rejects.toThrow(/ttl boom/);
+
+      // Nothing was registered, so the cleanup's DeleteTable leaves no orphan.
+      expect(registerInputs()).toEqual([]);
+    });
+
+    it('does not fail the deploy when auto-scaling registration itself throws', async () => {
+      // Best-effort contract: a table that AWS created successfully must not
+      // be destroyed by the partial-create cleanup over a scaling-target
+      // problem. `applyAutoScalingDiff` swallows send errors, so this pins
+      // the surrounding guard by making the client factory itself reject.
+      const boom = new Error('region resolution failed');
+      const originalRegion = mockSend.getMockImplementation();
+      void originalRegion;
+      const provider2 = new DynamoDBGlobalTableProvider();
+      (
+        provider2 as unknown as { getLocalAutoScalingClient: () => Promise<never> }
+      ).getLocalAutoScalingClient = () => Promise.reject(boom);
+
+      const result = await provider2.create('Prov', RESOURCE_TYPE, AUTOSCALED_PROPS);
+
+      expect(result.physicalId).toBe(TABLE_NAME);
+      expect(
+        mockSend.mock.calls.map((c) => c[0]).some((c) => c instanceof DeleteTableCommand)
+      ).toBe(false);
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Auto-scaling registration failed'));
+    });
+
     it('registers nothing on a PAY_PER_REQUEST table (AWS rejects targets there)', async () => {
       await provider.create('Prov', RESOURCE_TYPE, {
         ...AUTOSCALED_PROPS,
@@ -481,6 +544,59 @@ describe('DynamoDBGlobalTable per-index auto-scaling (issue #1419)', () => {
       expect(gone).toContainEqual([`table/${TABLE_NAME}`, 'dynamodb:table:ReadCapacityUnits']);
     });
 
+    it("deregisters a cross-region replica's index targets using the TABLE's index list", async () => {
+      // A replica that inherits throughput can come back with its own
+      // `GlobalSecondaryIndexes` list OMITTED (the per-replica entry is
+      // "replica-specific settings", and its ProvisionedThroughputOverride is
+      // documented as "if not described, uses the source table's"). Reading
+      // index names off the replica would then leak every
+      // dynamodb:index:ReadCapacityUnits target in that region past
+      // DeleteTable. Index names are identical across replicas, so the
+      // table-level list is the correct source.
+      describeOnceThenGone({
+        TableName: TABLE_NAME,
+        TableArn: TABLE_ARN,
+        TableStatus: 'ACTIVE',
+        GlobalSecondaryIndexes: [{ IndexName: 'gsi1' }],
+        Replicas: [
+          { RegionName: 'us-east-1' },
+          // No GlobalSecondaryIndexes key at all — the inheriting shape.
+          { RegionName: 'eu-west-1' },
+        ],
+      });
+
+      await provider.delete('Prov', TABLE_NAME, RESOURCE_TYPE, AUTOSCALED_PROPS);
+
+      expect(deregistered()).toContainEqual([
+        `table/${TABLE_NAME}/index/gsi1`,
+        'dynamodb:index:ReadCapacityUnits',
+      ]);
+      expect(autoScalingRegionSpy).toHaveBeenCalledWith('eu-west-1');
+    });
+
+    it('stays silent when a target was never registered (ObjectNotFoundException suppression)', async () => {
+      // delete() tears down index dimensions for every table that has GSIs,
+      // including tables that never had auto-scaling at all. AWS answers
+      // ObjectNotFoundException for those, which must be suppressed — an
+      // unsuppressed one would print two WARN lines per index on every
+      // destroy of an ordinary table.
+      describeOnceThenGone({
+        TableName: TABLE_NAME,
+        TableArn: TABLE_ARN,
+        TableStatus: 'ACTIVE',
+        GlobalSecondaryIndexes: [{ IndexName: 'gsi1' }],
+        Replicas: [{ RegionName: 'us-east-1' }],
+      });
+      const notFound = new Error('No scaling policy found for service namespace: dynamodb');
+      notFound.name = 'ObjectNotFoundException';
+      mockAutoScalingSend.mockReset();
+      mockAutoScalingSend.mockRejectedValue(notFound);
+
+      await provider.delete('Prov', TABLE_NAME, RESOURCE_TYPE, AUTOSCALED_PROPS);
+
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+
     it('takes index names from the live DescribeTable, not from the (possibly stale) template', async () => {
       describeOnceThenGone({
         TableName: TABLE_NAME,