Skip to content

Commit d57d4c9

Browse files
sbaker617Claude
andcommitted
consultopo: make retry backoff context-aware
Replace time.Sleep with a select on ctx.Done and time.After so that retries abort promptly when the caller's context is canceled or expired. Pass context from callers in file.go, directory.go, and election.go via QueryOptions so the retry loop can observe it. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Steve Baker <s.baker@slack-corp.com>
1 parent faf6c2e commit d57d4c9

5 files changed

Lines changed: 53 additions & 16 deletions

File tree

go/vt/topo/consultopo/directory.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,11 @@ limitations under the License.
1717
package consultopo
1818

1919
import (
20+
"context"
2021
"path"
2122
"strings"
2223

23-
"context"
24+
"github.com/hashicorp/consul/api"
2425

2526
"vitess.io/vitess/go/vt/topo"
2627
)
@@ -36,7 +37,7 @@ func (s *Server) ListDir(ctx context.Context, dirPath string, full bool) ([]topo
3637

3738
isRoot := dirPath == "" || dirPath == "/"
3839

39-
keys, _, err := s.kv.Keys(nodePath, "", nil)
40+
keys, _, err := s.kv.Keys(nodePath, "", (&api.QueryOptions{}).WithContext(ctx))
4041
if err != nil {
4142
return nil, err
4243
}

go/vt/topo/consultopo/election.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (mp *consulLeaderParticipation) Stop() {
123123
// GetCurrentLeaderID is part of the topo.LeaderParticipation interface
124124
func (mp *consulLeaderParticipation) GetCurrentLeaderID(ctx context.Context) (string, error) {
125125
electionPath := path.Join(mp.s.root, electionsPath, mp.name)
126-
pair, _, err := mp.s.kv.Get(electionPath, nil)
126+
pair, _, err := mp.s.kv.Get(electionPath, (&api.QueryOptions{}).WithContext(ctx))
127127
if err != nil {
128128
return "", err
129129
}

go/vt/topo/consultopo/file.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ limitations under the License.
1717
package consultopo
1818

1919
import (
20-
"path"
21-
2220
"context"
21+
"path"
2322

2423
"github.com/hashicorp/consul/api"
2524

@@ -41,7 +40,7 @@ func (s *Server) Create(ctx context.Context, filePath string, contents []byte) (
4140
Index: 0,
4241
},
4342
}
44-
ok, resp, _, err := s.kv.Txn(ops, nil)
43+
ok, resp, _, err := s.kv.Txn(ops, (&api.QueryOptions{}).WithContext(ctx))
4544
if err != nil {
4645
// Communication error.
4746
return nil, err
@@ -70,7 +69,7 @@ func (s *Server) Update(ctx context.Context, filePath string, contents []byte, v
7069
ops[0].Verb = api.KVCAS
7170
ops[0].Index = uint64(version.(ConsulVersion))
7271
}
73-
ok, resp, _, err := s.kv.Txn(ops, nil)
72+
ok, resp, _, err := s.kv.Txn(ops, (&api.QueryOptions{}).WithContext(ctx))
7473
if err != nil {
7574
// Communication error.
7675
return nil, err
@@ -87,7 +86,7 @@ func (s *Server) Update(ctx context.Context, filePath string, contents []byte, v
8786
func (s *Server) Get(ctx context.Context, filePath string) ([]byte, topo.Version, error) {
8887
nodePath := path.Join(s.root, filePath)
8988

90-
pair, _, err := s.kv.Get(nodePath, nil)
89+
pair, _, err := s.kv.Get(nodePath, (&api.QueryOptions{}).WithContext(ctx))
9190
if err != nil {
9291
return nil, nil, err
9392
}
@@ -107,7 +106,7 @@ func (s *Server) GetVersion(ctx context.Context, filePath string, version int64)
107106
func (s *Server) List(ctx context.Context, filePathPrefix string) ([]topo.KVInfo, error) {
108107
nodePathPrefix := path.Join(s.root, filePathPrefix)
109108

110-
pairs, _, err := s.kv.List(nodePathPrefix, nil)
109+
pairs, _, err := s.kv.List(nodePathPrefix, (&api.QueryOptions{}).WithContext(ctx))
111110
if err != nil {
112111
return []topo.KVInfo{}, err
113112
}
@@ -150,7 +149,7 @@ func (s *Server) Delete(ctx context.Context, filePath string, version topo.Versi
150149
ops[1].Verb = api.KVDeleteCAS
151150
ops[1].Index = uint64(version.(ConsulVersion))
152151
}
153-
ok, resp, _, err := s.kv.Txn(ops, nil)
152+
ok, resp, _, err := s.kv.Txn(ops, (&api.QueryOptions{}).WithContext(ctx))
154153
if err != nil {
155154
// Communication error.
156155
return err

go/vt/topo/consultopo/retry.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ func newRetryKV(inner kvClient, count int, baseDelay, maxDelay time.Duration, en
6868
func (r *retryKV) Get(key string, q *api.QueryOptions) (*api.KVPair, *api.QueryMeta, error) {
6969
var pair *api.KVPair
7070
var meta *api.QueryMeta
71-
err := r.retry(func() error {
71+
ctx := contextFromQueryOptions(q)
72+
err := r.retry(ctx, func() error {
7273
var ierr error
7374
pair, meta, ierr = r.inner.Get(key, q)
7475
return ierr
@@ -79,7 +80,8 @@ func (r *retryKV) Get(key string, q *api.QueryOptions) (*api.KVPair, *api.QueryM
7980
func (r *retryKV) List(prefix string, q *api.QueryOptions) (api.KVPairs, *api.QueryMeta, error) {
8081
var pairs api.KVPairs
8182
var meta *api.QueryMeta
82-
err := r.retry(func() error {
83+
ctx := contextFromQueryOptions(q)
84+
err := r.retry(ctx, func() error {
8385
var ierr error
8486
pairs, meta, ierr = r.inner.List(prefix, q)
8587
return ierr
@@ -90,7 +92,8 @@ func (r *retryKV) List(prefix string, q *api.QueryOptions) (api.KVPairs, *api.Qu
9092
func (r *retryKV) Keys(prefix string, separator string, q *api.QueryOptions) ([]string, *api.QueryMeta, error) {
9193
var keys []string
9294
var meta *api.QueryMeta
93-
err := r.retry(func() error {
95+
ctx := contextFromQueryOptions(q)
96+
err := r.retry(ctx, func() error {
9497
var ierr error
9598
keys, meta, ierr = r.inner.Keys(prefix, separator, q)
9699
return ierr
@@ -102,15 +105,23 @@ func (r *retryKV) Txn(txn api.KVTxnOps, q *api.QueryOptions) (bool, *api.KVTxnRe
102105
var ok bool
103106
var resp *api.KVTxnResponse
104107
var meta *api.QueryMeta
105-
err := r.retry(func() error {
108+
ctx := contextFromQueryOptions(q)
109+
err := r.retry(ctx, func() error {
106110
var ierr error
107111
ok, resp, meta, ierr = r.inner.Txn(txn, q)
108112
return ierr
109113
})
110114
return ok, resp, meta, err
111115
}
112116

113-
func (r *retryKV) retry(action func() error) error {
117+
func contextFromQueryOptions(q *api.QueryOptions) context.Context {
118+
if q != nil && q.Context() != nil {
119+
return q.Context()
120+
}
121+
return context.Background()
122+
}
123+
124+
func (r *retryKV) retry(ctx context.Context, action func() error) error {
114125
if !r.enabled {
115126
return action()
116127
}
@@ -119,7 +130,11 @@ func (r *retryKV) retry(action func() error) error {
119130
for attempt := 0; attempt < r.count; attempt++ {
120131
if attempt > 0 {
121132
delay := r.backoff(attempt)
122-
time.Sleep(delay)
133+
select {
134+
case <-ctx.Done():
135+
return ctx.Err()
136+
case <-time.After(delay):
137+
}
123138
}
124139

125140
err = action()

go/vt/topo/consultopo/retry_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,28 @@ func TestRetryKV_Backoff(t *testing.T) {
246246
assert.LessOrEqual(t, d10, r.maxDelay+r.maxDelay/4)
247247
}
248248

249+
func TestRetryKV_Get_ContextCanceledDuringBackoff(t *testing.T) {
250+
ctx, cancel := context.WithCancel(context.Background())
251+
mock := &mockKV{
252+
getFunc: func(call int) (*api.KVPair, *api.QueryMeta, error) {
253+
if call == 1 {
254+
cancel()
255+
}
256+
return nil, nil, errors.New("Unexpected response code: 500")
257+
},
258+
}
259+
r := newRetryKV(mock, 3, 500*time.Millisecond, 5*time.Second, true)
260+
261+
opts := (&api.QueryOptions{}).WithContext(ctx)
262+
start := time.Now()
263+
_, _, err := r.Get("test", opts)
264+
elapsed := time.Since(start)
265+
266+
assert.ErrorIs(t, err, context.Canceled)
267+
assert.Equal(t, 1, mock.getCalls)
268+
assert.Less(t, elapsed, 200*time.Millisecond)
269+
}
270+
249271
func TestRetryKV_Backoff_ZeroBaseDelay(t *testing.T) {
250272
r := &retryKV{
251273
baseDelay: 0,

0 commit comments

Comments
 (0)