Skip to content

Commit ee181da

Browse files
authored
fix(configclient): include fieldChecksums in doWrite retry gate
SetMany/SetManyTyped with WithFieldChecksums never retried because the gate only checked idempotencyKey and expectedChecksum. Per-field checksums make batch writes idempotent, so they now also opt in to retry when WithRetry is enabled. Closes #814
1 parent 640c2ad commit ee181da

2 files changed

Lines changed: 44 additions & 4 deletions

File tree

sdk/configclient/client_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,43 @@ func (t *countingWriteTransport) SetField(_ context.Context, _ *SetFieldRequest)
13661366
return &SetFieldResponse{}, nil
13671367
}
13681368

1369+
// countingBatchWriteTransport is a minimal Transport that fails the first N
1370+
// SetFields calls with a RetryableError and succeeds thereafter.
1371+
type countingBatchWriteTransport struct {
1372+
mockTransport
1373+
failUntil int
1374+
calls int
1375+
}
1376+
1377+
func (t *countingBatchWriteTransport) SetFields(_ context.Context, _ *SetFieldsRequest) (*SetFieldsResponse, error) {
1378+
t.calls++
1379+
if t.calls <= t.failUntil {
1380+
return nil, &RetryableError{Err: fmt.Errorf("unavailable")}
1381+
}
1382+
return &SetFieldsResponse{}, nil
1383+
}
1384+
1385+
func TestDoWrite_FieldChecksums_Retries(t *testing.T) {
1386+
ctx := context.Background()
1387+
tr := &countingBatchWriteTransport{failUntil: 2}
1388+
client := New(tr, WithRetry(RetryConfig{
1389+
MaxAttempts: 3,
1390+
InitialBackoff: time.Millisecond,
1391+
MaxBackoff: 10 * time.Millisecond,
1392+
RetryableCheck: IsRetryable,
1393+
}))
1394+
1395+
err := client.SetMany(ctx, "t1", map[string]string{"field": "value"}, "",
1396+
WithFieldChecksums(map[string]string{"field": "chk-abc"}),
1397+
)
1398+
if err != nil {
1399+
t.Fatalf("unexpected error: %v", err)
1400+
}
1401+
if tr.calls != 3 {
1402+
t.Errorf("SetMany with WithFieldChecksums should retry: got %d calls, want 3", tr.calls)
1403+
}
1404+
}
1405+
13691406
// --- WriteOption tests ---
13701407

13711408
func TestSet_WithDescription(t *testing.T) {

sdk/configclient/write.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,14 @@ func WithFieldChecksums(checksums map[string]string) WriteOption {
7575
return func(o *writeOptions) { o.fieldChecksums = checksums }
7676
}
7777

78-
// doWrite executes a write operation. Without an idempotency key or expected
79-
// checksum, the call is made exactly once. With either (both make the write
80-
// idempotent) and retry enabled on the client, transient errors trigger retry.
78+
// doWrite executes a write operation. Without an idempotency key, expected
79+
// checksum, or field checksums, the call is made exactly once. With any of
80+
// those options (all make the write idempotent) and retry enabled on the
81+
// client, transient errors trigger retry. [WithFieldChecksums] on batch writes
82+
// (SetMany/SetManyTyped) also enables retry because per-field checksums make
83+
// the operation idempotent.
8184
func doWrite(ctx context.Context, c *Client, wo writeOptions, fn func(ctx context.Context) error) error {
82-
if (wo.idempotencyKey != "" || wo.expectedChecksum != "") && c.opts.retryEnabled {
85+
if (wo.idempotencyKey != "" || wo.expectedChecksum != "" || len(wo.fieldChecksums) > 0) && c.opts.retryEnabled {
8386
return retryDo(ctx, c, fn)
8487
}
8588
return fn(ctx)

0 commit comments

Comments
 (0)