diff --git a/CHANGELOG.md b/CHANGELOG.md index 47dbba996eb..3287db55cad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ * [BUGFIX] Querier: Fix parquet queryable fallback returning a nil error instead of the actual query error in `LabelValues` and `LabelNames`. #7638 * [BUGFIX] Storage: Default the Azure `endpoint_suffix` to `blob.core.windows.net` instead of empty. Cortex builds the Thanos Azure config directly and bypasses Thanos' default, so an unset suffix produced an invalid FQDN (`.`) and components hung on startup with DNS errors. #5449 * [BUGFIX] Store Gateway: Fix misleading "no index cache backend addresses" validation error being reported for chunks-cache, metadata-cache, and parquet caches when their memcached or redis backend is configured without addresses. The message is now the cache-type-agnostic "no cache backend addresses". #7675 +* [BUGFIX] Ring: Fix DynamoDB KV CAS not retrying on transactional conditional check failures. `TransactWriteItems` reports condition failures as `TransactionCanceledException` with a `ConditionalCheckFailed` cancellation reason, which was not recognized as retryable, so any concurrent ring update conflict (e.g. many ingesters joining during a rolling update) failed immediately instead of re-reading and retrying. `TransactionConflict` cancellation reasons are also treated as retryable. #7706 ## 1.21.0 2026-04-24 diff --git a/pkg/ring/kv/dynamodb/dynamodb.go b/pkg/ring/kv/dynamodb/dynamodb.go index 9bc4e99e30d..dc33301851c 100644 --- a/pkg/ring/kv/dynamodb/dynamodb.go +++ b/pkg/ring/kv/dynamodb/dynamodb.go @@ -265,8 +265,7 @@ func (kv dynamodbKV) Batch(ctx context.Context, put map[dynamodbKey]dynamodbItem TransactItems: slice, }) if err != nil { - var checkFailed *types.ConditionalCheckFailedException - isCheckFailed := errors.As(err, &checkFailed) + isCheckFailed := isConditionalCheckFailure(err) if isCheckFailed { kv.logger.Log("msg", "conditional check failed on DynamoDB Batch", "err", err) } @@ -280,6 +279,38 @@ func (kv dynamodbKV) Batch(ctx context.Context, put map[dynamodbKey]dynamodbItem return totalCapacity, false, nil } +// isConditionalCheckFailure returns true if the given error was caused by a +// conditional check failure, meaning another writer updated the item between +// our read and write. Such conflicts are retryable: the caller should re-read +// the latest state and retry the operation. +// +// DynamoDB reports conditional check failures differently depending on the API: +// - Single-item operations (PutItem, UpdateItem, DeleteItem) return +// ConditionalCheckFailedException. +// - TransactWriteItems returns TransactionCanceledException with the failure +// reported in CancellationReasons with code "ConditionalCheckFailed". +// +// TransactionConflict cancellation reasons (another transaction is in progress +// on the same item) are equally transient and also treated as retryable. +func isConditionalCheckFailure(err error) bool { + if _, ok := errors.AsType[*types.ConditionalCheckFailedException](err); ok { + return true + } + + if txnCanceled, ok := errors.AsType[*types.TransactionCanceledException](err); ok { + for _, reason := range txnCanceled.CancellationReasons { + if reason.Code == nil { + continue + } + switch *reason.Code { + case "ConditionalCheckFailed", "TransactionConflict": + return true + } + } + } + return false +} + func (kv dynamodbKV) generatePutItemRequest(key dynamodbKey, ddbItem dynamodbItem) map[string]types.AttributeValue { item := generateItemKey(key) item[contentData] = &types.AttributeValueMemberB{Value: ddbItem.data} diff --git a/pkg/ring/kv/dynamodb/dynamodb_test.go b/pkg/ring/kv/dynamodb/dynamodb_test.go index 065e38c6442..54419c603ec 100644 --- a/pkg/ring/kv/dynamodb/dynamodb_test.go +++ b/pkg/ring/kv/dynamodb/dynamodb_test.go @@ -170,6 +170,38 @@ func Test_Batch_Error(t *testing.T) { mockError: &types.ConditionalCheckFailedException{}, expectedRetry: true, }, + { + // TransactWriteItems reports conditional check failures as + // TransactionCanceledException with a ConditionalCheckFailed + // cancellation reason, wrapped by the SDK operation error. + name: "transaction_canceled_conditional_check_failed_should_retry", + mockError: fmt.Errorf("operation error DynamoDB: TransactWriteItems: %w", &types.TransactionCanceledException{ + Message: aws.String("Transaction cancelled, please refer cancellation reasons for specific reasons"), + CancellationReasons: []types.CancellationReason{ + {Code: aws.String("None")}, + {Code: aws.String("ConditionalCheckFailed")}, + }, + }), + expectedRetry: true, + }, + { + name: "transaction_canceled_transaction_conflict_should_retry", + mockError: fmt.Errorf("operation error DynamoDB: TransactWriteItems: %w", &types.TransactionCanceledException{ + CancellationReasons: []types.CancellationReason{ + {Code: aws.String("TransactionConflict")}, + }, + }), + expectedRetry: true, + }, + { + name: "transaction_canceled_other_reason_no_retry", + mockError: fmt.Errorf("operation error DynamoDB: TransactWriteItems: %w", &types.TransactionCanceledException{ + CancellationReasons: []types.CancellationReason{ + {Code: aws.String("ValidationError")}, + }, + }), + expectedRetry: false, + }, } for _, tc := range testCases {