Skip to content

Commit 94283d9

Browse files
oten91claude
andcommitted
fix: tag hedge relays separately so geography doesn't punish slow suppliers
The hedge race fan-out was double-counting in path_relays_total: both the primary AND the hedge branch's relay completed and called RecordRelay with request_type="normal", inflating the visible RPS for whichever endpoint happens to be picked as hedge most often (typically the lowest-latency one). Symptom observed on canary at 22m post-deploy of #512: rpcgate.xyz with 1 endpoint at 431 RPS while stakenodes.org with 21 endpoints averaged 7.7 RPS per endpoint. Per-endpoint share was 17x off baseline despite uniform-random tier-1 selection. The skew traced to (a) hedge metric double-count and (b) hedge losers getting latency-penalty signals for slowness that only mattered inside the race they already lost. This change: - Adds RelayTypeHedge metric constant - Adds MarkAsHedge() to ProtocolRequestContext interface - Hedge branch is tagged before HandleServiceRequest; protocol layer records the relay under request_type="hedge" instead of "normal" - Skips recordLatencyPenaltySignalsIfNeeded for hedge branches — far-from- gateway endpoints no longer accumulate slow-response reputation penalties for losing hedge races they only entered because of distance - Reputation success/error signals still fire so endpoints get fair feedback about whether they actually work Dashboards filtering request_type="normal" by default now show fair primary- traffic distribution. Operators can flip the variable to "hedge" to see the backup-attempt side independently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dc8c117 commit 94283d9

5 files changed

Lines changed: 96 additions & 4 deletions

File tree

gateway/hedge.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,12 @@ func (hr *hedgeRacer) race(
211211
hedgeDetachedCtx, hedgeDetachedCancel := detachedHedgeCtx(ctx, defaultLoserGraceWindow)
212212
hr.hedgeReqCancel = hedgeDetachedCancel
213213
hedgeCtx.SetParentContext(hedgeDetachedCtx)
214+
// Tag this branch as hedge so the protocol records it under
215+
// request_type="hedge" and skips latency-penalty reputation signals.
216+
// Without this, fast endpoints picked as hedge accumulate inflated
217+
// "normal" relay counts and slow endpoints get latency-penalized for
218+
// losing races they only entered because of distance.
219+
hedgeCtx.MarkAsHedge()
214220
// Start hedge request
215221
go hr.executeRequest(ctx, hedgeCtx, hedgeEndpoint, hedgeSupplier, true, 2, time.Now())
216222
}

gateway/protocol.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,16 @@ type ProtocolRequestContext interface {
179179
// than being RST'd the moment the winner is picked and the caller's handler unwinds.
180180
SetParentContext(ctx context.Context)
181181

182+
// MarkAsHedge tags this request as the hedge branch of a hedge race so that
183+
// (1) its relay metric is recorded with request_type="hedge" rather than "normal"
184+
// — keeps per-domain RPS dashboards honest by not double-counting fast endpoints
185+
// that win disproportionate hedge races, and
186+
// (2) latency-penalty reputation signals are skipped — slow geographies should not
187+
// be penalized for losing hedge races they only entered because of distance.
188+
// Reputation success/error signals still fire so endpoints get fair feedback
189+
// about whether they actually work.
190+
MarkAsHedge()
191+
182192
// GetObservations builds and returns the set of protocol-specific observations using the current context.
183193
//
184194
// Hypothetical illustrative example.

metrics/metrics.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,12 @@ const (
500500
RelayTypeNormal = "normal"
501501
RelayTypeHealthCheck = "health_check"
502502
RelayTypeProbation = "probation"
503+
// RelayTypeHedge tags relays issued as the hedge branch of a hedge race —
504+
// a backup attempt to a different endpoint when the primary is slow. These
505+
// inflate per-domain RPS metrics if grouped with normal traffic, because
506+
// fast endpoints are picked as hedge again and again. Dashboards filter to
507+
// request_type="normal" by default to show fair primary-traffic distribution.
508+
RelayTypeHedge = "hedge"
503509
)
504510

505511
var RelaysTotal = promauto.NewCounterVec(

protocol/shannon/context.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,15 @@ type requestContext struct {
135135
// When nil, no reputation-based filtering is applied.
136136
reputationService reputation.ReputationService
137137

138+
// isHedge marks this request as the hedge branch of a hedge race. Set by
139+
// MarkAsHedge() before HandleServiceRequest is called. Used at metric/reputation
140+
// recording time to:
141+
// - Tag the relay metric as request_type="hedge" (not "normal")
142+
// - Skip latency-penalty reputation signals (don't punish far-from-gateway endpoints)
143+
// Success/error reputation signals still fire — endpoints still get fair feedback
144+
// about whether they actually work.
145+
isHedge bool
146+
138147
// tieredSelector provides access to tier-based selection and probation status.
139148
// Used to check if endpoints are in probation when recording success signals.
140149
// If non-nil and endpoint is in probation, RecoverySuccessSignal is used instead of SuccessSignal.
@@ -441,6 +450,13 @@ func (rc *requestContext) SetParentContext(ctx context.Context) {
441450
rc.context = ctx
442451
}
443452

453+
// MarkAsHedge tags this request as the hedge branch of a hedge race.
454+
// Implements gateway.ProtocolRequestContext. See the field doc on `isHedge`
455+
// for behavior implications (metric request_type, latency penalty skip).
456+
func (rc *requestContext) MarkAsHedge() {
457+
rc.isHedge = true
458+
}
459+
444460
// getSelectedEndpoint returns the currently selected endpoint in a thread-safe manner.
445461
func (rc *requestContext) getSelectedEndpoint() endpoint {
446462
rc.selectedEndpointMutex.RLock()
@@ -1212,8 +1228,12 @@ func (rc *requestContext) handleEndpointError(
12121228
statusCodeStr = metrics.GetStatusCodeCategory(statusCode)
12131229
}
12141230

1215-
// Determine relay type - errors are recorded as normal type (probation detection requires success)
1231+
// Determine relay type - errors are recorded as normal type (probation detection requires success).
1232+
// Hedge errors are tagged as hedge so they don't pollute the per-domain RPS view.
12161233
relayType := metrics.RelayTypeNormal
1234+
if rc.isHedge {
1235+
relayType = metrics.RelayTypeHedge
1236+
}
12171237
metrics.RecordRelay(domain, rpcTypeStr, string(rc.serviceID), statusCodeStr, reputationSignal, relayType, latency.Seconds())
12181238

12191239
// Return the original response (preserving any response bytes) with the error.
@@ -1291,6 +1311,13 @@ func (rc *requestContext) handleEndpointSuccess(
12911311
Str("endpoint", string(selectedEndpointAddr)).
12921312
Str("service_id", string(rc.serviceID)).
12931313
Msg("Backend returned 5xx error - recording critical error signal for reputation")
1314+
} else if rc.isHedge {
1315+
// Hedge branch succeeded — record success signal (so the endpoint gets fair
1316+
// reputation feedback) but tag the relay metric as hedge so it doesn't
1317+
// inflate per-domain RPS dashboards. Latency penalty is skipped below.
1318+
signal = reputation.NewSuccessSignal(latency)
1319+
reputationSignal = metrics.SignalOK
1320+
relayType = metrics.RelayTypeHedge
12941321
} else if rc.tieredSelector != nil && rc.tieredSelector.Config().Probation.Enabled && rc.tieredSelector.IsInProbation(endpointKey) {
12951322
// Probation endpoint succeeded - use recovery signal for stronger positive reinforcement
12961323
signal = reputation.NewRecoverySuccessSignal(latency)
@@ -1321,8 +1348,11 @@ func (rc *requestContext) handleEndpointSuccess(
13211348
rc.logger.Warn().Err(err).Msg("Failed to record reputation signal for success")
13221349
}
13231350

1324-
// Record additional latency penalty signals if applicable (only for actual successes)
1325-
if !isBackend5xx {
1351+
// Record additional latency penalty signals if applicable (only for actual successes).
1352+
// Skip for hedge branches: a hedge that's "slow" only failed to win the race —
1353+
// often because of network distance, not endpoint health. Penalizing it for
1354+
// latency would systematically drift far-from-gateway endpoints downward.
1355+
if !isBackend5xx && !rc.isHedge {
13261356
rc.recordLatencyPenaltySignalsIfNeeded(endpointKey, latency)
13271357
}
13281358
} else {
@@ -1332,7 +1362,11 @@ func (rc *requestContext) handleEndpointSuccess(
13321362
} else {
13331363
reputationSignal = metrics.SignalOK
13341364
}
1335-
relayType = metrics.RelayTypeNormal
1365+
if rc.isHedge {
1366+
relayType = metrics.RelayTypeHedge
1367+
} else {
1368+
relayType = metrics.RelayTypeNormal
1369+
}
13361370
}
13371371

13381372
// Record relay metric for successful request
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package shannon
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/pokt-network/path/gateway"
9+
)
10+
11+
// TestRequestContext_SatisfiesProtocolRequestContext is a compile-time guarantee
12+
// that *requestContext implements gateway.ProtocolRequestContext, including the
13+
// new MarkAsHedge method. If this stops compiling, the interface drifted.
14+
func TestRequestContext_SatisfiesProtocolRequestContext(t *testing.T) {
15+
var _ gateway.ProtocolRequestContext = (*requestContext)(nil)
16+
}
17+
18+
// TestMarkAsHedge_SetsFlag verifies that MarkAsHedge flips the isHedge bit, which
19+
// is what later gates the relay-type tagging and the latency-penalty skip in
20+
// handleEndpointSuccess / handleEndpointError.
21+
func TestMarkAsHedge_SetsFlag(t *testing.T) {
22+
rc := &requestContext{}
23+
require.False(t, rc.isHedge, "default should be false (non-hedge)")
24+
rc.MarkAsHedge()
25+
require.True(t, rc.isHedge, "MarkAsHedge must set isHedge=true so downstream relay/reputation paths can branch on it")
26+
}
27+
28+
// TestMarkAsHedge_Idempotent verifies that calling MarkAsHedge twice is a no-op
29+
// (no panic, flag stays true). This matters because hedge.go currently calls it
30+
// once but a future caller might call it defensively.
31+
func TestMarkAsHedge_Idempotent(t *testing.T) {
32+
rc := &requestContext{}
33+
rc.MarkAsHedge()
34+
rc.MarkAsHedge()
35+
require.True(t, rc.isHedge)
36+
}

0 commit comments

Comments
 (0)