Skip to content

Commit 4a94686

Browse files
committed
feat(ehancement): make fair context deadline for all hedged requests
1 parent 91cb499 commit 4a94686

2 files changed

Lines changed: 59 additions & 8 deletions

File tree

hedging.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,30 @@ func (ht *Hedging) RoundTrip(req *http.Request) (*http.Response, error) {
166166
}
167167

168168
ctx := req.Context()
169-
hedgeCtx, cancel := context.WithCancel(ctx)
169+
deadline, hasDeadline := ctx.Deadline()
170+
171+
// Derive hedgeCtx from the original request context to respect cancellations
172+
var (
173+
hedgeCtx context.Context
174+
cancel context.CancelFunc
175+
)
176+
if hasDeadline {
177+
// Use original deadline for the race (first to complete wins)
178+
remaining := time.Until(deadline)
179+
if remaining > 0 {
180+
hedgeCtx, cancel = context.WithTimeout(ctx, remaining)
181+
} else {
182+
// Deadline already expired, use context with cancel
183+
hedgeCtx, cancel = context.WithCancel(ctx)
184+
}
185+
} else {
186+
// No deadline in original context, create cancellable context from it
187+
hedgeCtx, cancel = context.WithCancel(ctx)
188+
}
189+
190+
// defer cancel() ensures cleanup on all paths (timeout, cancellation, or normal return)
191+
// cancel() may also be called inside once.Do() when a request wins, but calling it
192+
// multiple times is safe and ensures the context is canceled as soon as any goroutine completes
170193
defer cancel()
171194

172195
type result struct {
@@ -177,21 +200,24 @@ func (ht *Hedging) RoundTrip(req *http.Request) (*http.Response, error) {
177200
resultCh := make(chan result, ht.MaxRequest())
178201
var once sync.Once
179202

180-
for i := 0; i < ht.MaxRequest(); i++ {
203+
ht.lock.RLock()
204+
maxReq := ht.maxRequest
205+
delay := ht.delay
206+
rateDelay := ht.rateDelay
207+
ht.lock.RUnlock()
208+
209+
for i := 0; i < maxReq; i++ {
181210
if i > 0 {
182-
if ht.Delay() > 0 {
211+
if delay > 0 {
183212
select {
184-
case <-time.After(ht.Delay()):
213+
case <-time.After(delay):
185214
case <-hedgeCtx.Done():
186215
break
187216
}
188217
}
189218

190219
// Rate limiting: add delay between requests based on maxPerSecond
191220
// to prevent overwhelming the server.
192-
ht.lock.RLock()
193-
rateDelay := ht.rateDelay
194-
ht.lock.RUnlock()
195221
if rateDelay > 0 {
196222
select {
197223
case <-time.After(rateDelay):
@@ -202,13 +228,16 @@ func (ht *Hedging) RoundTrip(req *http.Request) (*http.Response, error) {
202228
}
203229

204230
go func() {
205-
hedgedReq := req.Clone(ctx)
231+
hedgedReq := req.Clone(hedgeCtx)
206232
resp, err := ht.transport.RoundTrip(hedgedReq)
207233

208234
won := false
209235
once.Do(func() {
210236
won = true
211237
resultCh <- result{resp: resp, err: err}
238+
239+
// Cancel inside once.Do() to stop other goroutines immediately when a request wins
240+
// defer cancel() ensures cleanup even if no request completes successfully
212241
cancel()
213242
})
214243

hedging_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,3 +704,25 @@ func TestHedgingNoDoubleWrap(t *testing.T) {
704704
// Verify the configuration is the new one
705705
assertEqual(t, hedging2.Delay(), 100*time.Millisecond, "Expected 100ms delay")
706706
}
707+
708+
func TestHedgingRoundTripDeadlineExpired(t *testing.T) {
709+
var attemptCount int32
710+
ts := createHedgingTestServer(t, &attemptCount, 0, 0)
711+
defer ts.Close()
712+
713+
h := NewHedging().
714+
SetDelay(10 * time.Millisecond).
715+
SetMaxRequest(3).
716+
SetMaxRequestPerSecond(0)
717+
718+
c := dcnl().SetHedging(h)
719+
720+
ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-1*time.Millisecond))
721+
defer cancel()
722+
723+
_, err := c.R().SetContext(ctx).Get(ts.URL + "/")
724+
assertErrorIs(t, context.DeadlineExceeded, err, "Expected context deadline expired error")
725+
726+
time.Sleep(50 * time.Millisecond)
727+
assertEqual(t, int32(0), atomic.LoadInt32(&attemptCount))
728+
}

0 commit comments

Comments
 (0)