Skip to content

Commit e774524

Browse files
committed
fix(hedging): adapt suggestions
Signed-off-by: Ahmet DEMIR <me@ahmet2mir.eu>
1 parent 14669c5 commit e774524

File tree

5 files changed

+249
-64
lines changed

5 files changed

+249
-64
lines changed

BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
"curl.go",
1414
"debug.go",
1515
"digest.go",
16+
"hedging.go",
1617
"load_balancer.go",
1718
"middleware.go",
1819
"multipart.go",
@@ -42,6 +43,7 @@ go_test(
4243
"context_test.go",
4344
"curl_test.go",
4445
"digest_test.go",
46+
"hedging_test.go",
4547
"load_balancer_test.go",
4648
"middleware_test.go",
4749
"multipart_test.go",

client.go

Lines changed: 31 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,6 @@ func (c *Client) RetryCount() int {
12641264
func (c *Client) SetRetryCount(count int) *Client {
12651265
c.lock.Lock()
12661266
defer c.lock.Unlock()
1267-
12681267
c.retryCount = count
12691268
return c
12701269
}
@@ -1443,109 +1442,101 @@ func (c *Client) isHedgingEnabled() bool {
14431442
func (c *Client) SetHedgingDelay(delay time.Duration) *Client {
14441443
c.lock.Lock()
14451444
defer c.lock.Unlock()
1446-
1447-
if !c.isHedgingEnabled() {
1448-
c.log.Errorf("SetHedgingDelay: %v", ErrHedgingDisabled)
1445+
if c.isHedgingEnabled() {
1446+
c.hedging.delay = delay
14491447
return c
14501448
}
1451-
1452-
c.hedging.delay = delay
1449+
c.log.Errorf("SetHedgingDelay: %v", ErrHedgingDisabled)
14531450
return c
14541451
}
14551452

14561453
// HedgingDelay method returns the configured hedging delay.
14571454
func (c *Client) HedgingDelay() time.Duration {
14581455
c.lock.RLock()
14591456
defer c.lock.RUnlock()
1460-
if !c.isHedgingEnabled() {
1461-
return hedgingDefaultDelay
1457+
if c.isHedgingEnabled() {
1458+
return c.hedging.delay
14621459
}
1463-
return c.hedging.delay
1460+
return hedgingDefaultDelay
14641461
}
14651462

14661463
// SetHedgingUpTo method sets maximum concurrent hedged requests.
14671464
func (c *Client) SetHedgingUpTo(upTo int) *Client {
14681465
c.lock.Lock()
14691466
defer c.lock.Unlock()
1470-
1471-
if !c.isHedgingEnabled() {
1472-
c.log.Errorf("SetHedgingUpTo: %v", ErrHedgingDisabled)
1467+
if c.isHedgingEnabled() {
1468+
c.hedging.upTo = upTo
14731469
return c
14741470
}
1475-
1476-
c.hedging.upTo = upTo
1471+
c.log.Errorf("SetHedgingUpTo: %v", ErrHedgingDisabled)
14771472
return c
14781473
}
14791474

14801475
// HedgingUpTo method returns the maximum concurrent requests.
14811476
func (c *Client) HedgingUpTo() int {
14821477
c.lock.RLock()
14831478
defer c.lock.RUnlock()
1484-
if !c.isHedgingEnabled() {
1485-
return hedgingDefaultUpTo
1479+
if c.isHedgingEnabled() {
1480+
return c.hedging.upTo
14861481
}
1487-
return c.hedging.upTo
1482+
return hedgingDefaultUpTo
14881483
}
14891484

14901485
// SetHedgingMaxPerSecond method sets rate limit for hedged requests.
14911486
func (c *Client) SetHedgingMaxPerSecond(maxPerSecond float64) *Client {
14921487
c.lock.Lock()
14931488
defer c.lock.Unlock()
1494-
1495-
if !c.isHedgingEnabled() {
1496-
c.log.Errorf("SetHedgingMaxPerSecond: %v", ErrHedgingDisabled)
1489+
if c.isHedgingEnabled() {
1490+
c.hedging.maxPerSecond = maxPerSecond
14971491
return c
14981492
}
1499-
1500-
c.hedging.maxPerSecond = maxPerSecond
1493+
c.log.Errorf("SetHedgingMaxPerSecond: %v", ErrHedgingDisabled)
15011494
return c
15021495
}
15031496

15041497
// HedgingMaxPerSecond method returns the hedging rate limit.
15051498
func (c *Client) HedgingMaxPerSecond() float64 {
15061499
c.lock.RLock()
15071500
defer c.lock.RUnlock()
1508-
if !c.isHedgingEnabled() {
1509-
return hedgingDefaultMaxPerSecond
1501+
if c.isHedgingEnabled() {
1502+
return c.hedging.maxPerSecond
15101503
}
1511-
return c.hedging.maxPerSecond
1504+
return hedgingDefaultMaxPerSecond
15121505
}
15131506

15141507
// SetHedgingAllowNonReadOnly method allows hedging for non-read-only HTTP methods.
15151508
// By default, only read-only methods (GET, HEAD, OPTIONS, TRACE) are hedged.
1516-
// Use this with caution as hedging write operations can lead to duplicates.
1509+
// NOTE:
1510+
// - Use this with caution as hedging write operations can lead to duplicates.
15171511
func (c *Client) SetHedgingAllowNonReadOnly(allow bool) *Client {
15181512
c.lock.Lock()
15191513
defer c.lock.Unlock()
15201514

1521-
if !c.isHedgingEnabled() {
1522-
c.log.Errorf("SetHedgingAllowNonReadOnly: %v", ErrHedgingDisabled)
1515+
if c.isHedgingEnabled() {
1516+
c.hedging.allowNonReadOnly = allow
1517+
// Re-wrap to apply new settings
1518+
c.unwrapHedgingTransport()
1519+
c.wrapTransportWithHedging()
15231520
return c
15241521
}
1525-
1526-
c.hedging.allowNonReadOnly = allow
1527-
1528-
// Re-wrap to apply new settings
1529-
c.unwrapHedgingTransport()
1530-
c.wrapTransportWithHedging()
1531-
1522+
c.log.Errorf("SetHedgingAllowNonReadOnly: %v", ErrHedgingDisabled)
15321523
return c
15331524
}
15341525

15351526
// IsHedgingAllowNonReadOnly method returns true if hedging is enabled for non-read-only methods.
15361527
func (c *Client) IsHedgingAllowNonReadOnly() bool {
15371528
c.lock.RLock()
15381529
defer c.lock.RUnlock()
1539-
if !c.isHedgingEnabled() {
1540-
return hedgingDefaultAllowNonReadOnly
1530+
if c.isHedgingEnabled() {
1531+
return c.hedging.allowNonReadOnly
15411532
}
1542-
return c.hedging.allowNonReadOnly
1533+
return hedgingDefaultAllowNonReadOnly
15431534
}
15441535

15451536
// EnableHedging method enables hedging with the given configuration.
15461537
//
15471538
// Hedging sends multiple concurrent requests with staggered delays and returns
1548-
// the first successful response to reduce tail latency. Only read-only HTTP methods
1539+
// the first response to complete to reduce tail latency. Only read-only HTTP methods
15491540
// (GET, HEAD, OPTIONS, TRACE) are hedged by default unless SetHedgingAllowNonReadOnly is used.
15501541
//
15511542
// client.EnableHedging(
@@ -1605,7 +1596,7 @@ func (c *Client) wrapTransportWithHedging() {
16051596

16061597
currentTransport := c.httpClient.Transport
16071598
if currentTransport == nil {
1608-
currentTransport = http.DefaultTransport
1599+
currentTransport = createTransport(nil, nil)
16091600
}
16101601

16111602
// Already set

hedging.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,13 @@ func (ht *hedgingTransport) RoundTrip(req *http.Request) (*http.Response, error)
9999
})
100100

101101
if !won && resp != nil && resp.Body != nil {
102-
closeq(resp.Body)
102+
drainReadCloser(resp.Body)
103103
}
104104
}()
105105
}
106106

107107
res := <-resultCh
108+
close(resultCh)
108109
return res.resp, res.err
109110
}
110111

0 commit comments

Comments
 (0)