Skip to content

Fix DynamoDB KV CAS not retrying on transactional conditional check failures#7706

Merged
yeya24 merged 2 commits into
cortexproject:masterfrom
yeya24:fix/ddb-kv-transact-conditional-check
Jul 20, 2026
Merged

Fix DynamoDB KV CAS not retrying on transactional conditional check failures#7706
yeya24 merged 2 commits into
cortexproject:masterfrom
yeya24:fix/ddb-kv-transact-conditional-check

Conversation

@yeya24

@yeya24 yeya24 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

What this PR does:

The Batch() error detection in the DynamoDB KV store only matches ConditionalCheckFailedException, which DynamoDB returns for single-item operations (PutItem/UpdateItem with a ConditionExpression). But Batch() calls TransactWriteItems, which reports condition failures as TransactionCanceledException with a ConditionalCheckFailed cancellation reason instead. TransactionCanceledException has no Unwrap() and the cancellation reasons are plain data fields, so errors.As(err, *ConditionalCheckFailedException) can never match on this code path — the SDK's TransactWriteItems error deserializer doesn't even have a ConditionalCheckFailedException case.

As a result retry=false is always returned for CAS write conflicts, and the CAS loop in client.go gives up on the first attempt instead of re-reading the ring and retrying with a fresh version. The 10-retry backoff loop (maxCasRetries) is effectively dead code for the most common conflict signal.

Production impact observed: during rolling updates where many ingesters cycle concurrently, a joining ingester that loses the optimistic-concurrency race on the ring key fails startup with:

module failed: ingester subservice failed: service ingester ring lifecycler failed: failed to pick tokens in the KV store, ring: ingester, state: ACTIVE: operation error DynamoDB: TransactWriteItems, https response error StatusCode: 400, ..., TransactionCanceledException: Transaction cancelled, please refer cancellation reasons for specific reasons [ConditionalCheckFailed]

and the process exits (crash + kubelet restart) instead of retrying. The failure occurs ~18ms after the CAS write — no backoff ever happens. The heartbeat path survives the same error only because its caller sleeps and retries at the next heartbeat interval.

The fix: extract the error classification into isConditionalCheckFailure(), which handles both shapes:

  • ConditionalCheckFailedException (single-item operations) — unchanged behavior
  • TransactionCanceledException with a ConditionalCheckFailed cancellation reason (transactions)

TransactionConflict cancellation reasons (another transaction in progress on the same item) are equally transient under concurrent ring writes and are also treated as retryable.

With this fix the CAS loop does what it was designed to do on conflict: re-read the latest ring state, re-apply the mutation, and write with the fresh version condition.

Which issue(s) this PR fixes:
N/A

Checklist

  • Tests updated
  • Documentation added (not needed — no config/API change)
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags (not needed — no new flags)

…ailures

The Batch() error detection in the DynamoDB KV store only matched
ConditionalCheckFailedException, which DynamoDB returns for single-item
operations. TransactWriteItems reports condition failures as
TransactionCanceledException with a ConditionalCheckFailed cancellation
reason, so errors.As never matched, retry=false was returned, and the
CAS loop gave up on the first attempt instead of re-reading the ring
and retrying.

In practice this means every ring write conflict on the DynamoDB KV
store is treated as fatal. During rolling updates where many ingesters
join/leave concurrently, a joining ingester that loses the optimistic
concurrency race fails startup with 'failed to pick tokens in the KV
store' and crashes, instead of retrying with a fresh read.

Detect TransactionCanceledException and inspect CancellationReasons for
ConditionalCheckFailed. TransactionConflict reasons (concurrent
transaction on the same item) are equally transient and also treated
as retryable.

Signed-off-by: Ben Ye <benye@amazon.com>
Signed-off-by: Ben Ye <benye@amazon.com>
@yeya24
yeya24 merged commit 4e9dffb into cortexproject:master Jul 20, 2026
74 of 75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants