Label balancing#210
Conversation
tonatoz
left a comment
There was a problem hiding this comment.
Solid feature — clean reuse of filterUpstreams, mirrors the existing RatingStrategy/failsafe-config patterns, with docs and focused unit tests. A few non-blocking suggestions:
1. Retry-delay default change — please scope or document it
The PR drops the implicit retry.delay = 300ms default and relaxes validation to < 0. Since internal/resilience/failsafe.go only applies backoff when Delay > 0, this means every user with retries but no explicit delay now gets zero-delay, back-to-back retries — not just label-balancing users. This is a global behavior change that isn't mentioned in the PR description (only the "Default: 300ms" doc line was removed).
Suggestion: keep the 300ms default and let label-balancing configs set delay: 0 explicitly. If the change is intentional, please call it out in the PR description / changelog and add a note in the retry docs, since it's effectively a breaking change for retry timing.
2. pass-on-error vs. hedging
pass-on-error advances the group cursor on every non-first SelectUpstream call, which assumes calls are sequential retries (one call = one error). With hedging enabled, the concurrent second attempt from the initial fan-out also sees firstCall == false and jumps to the next group even though no error occurred — so initial hedges get spread across groups.
Suggestion: document that pass-on-error is designed around retries (not concurrent hedges), or guard the cursor advance so it only triggers on an actual error path. Even a comment on SelectUpstream clarifying the assumption would help future readers.
3. Test coverage suggestions
The strategy-level and PartitionLabelGroups tests are good. A few additions would raise confidence:
NewLabelGroupStrategy(registry path): theGetSortedUpstreams → GetUpstream → GetGroupLabels → PartitionLabelGroupswiring (incl. theup == nilbranch) is currently untested — onlyNewLabelGroupStrategyWithGroupsis exercised.- End-to-end through
createStrategy/execution_flow: the actual feature behavior — falling through groups via retries — isn't covered at the flow level. include-default: falseat the strategy level (only covered in thePartitionLabelGroupstest today).- Concurrency: a test exercising hedge +
pass-on-errorwould lock down the behavior from point 2.
None of these block merge — point 1 is the one I'd most like a follow-up or a line in the description on.
Label-based balancing
Summary
Adds a second, optional balancing mode on top of the existing per-method rating.
Until now every request to a chain was balanced across all of that chain's upstreams
using the score-policy rating (latency, error rate, availability, …). This PR introduces
priority-group balancing: operators tag upstreams with
group-labels, declare anorderof those labels, and requests are served from the highest-priority group first,falling through to lower-priority groups when the current group can't serve. Rating still
decides ordering within a group.
Typical use case: prefer cheap
fullnodes, fall back toarchivenodes only when thefull group can't handle a request.
Configuration
label-balancingis configured as a global default underupstream-configand can beoverridden per chain under
chain-defaults.<chain>(same pattern asfailsafe-config).When neither is set, behavior is unchanged (pure rating).
Fields
orderlabel-balancingis set; entries must be non-empty and unique.pass-on-errorfalse).include-defaulttrue(default), upstreams carrying none of theorderlabels form a final fallback group tried after all configured groups. Whenfalse, they are excluded from routing while label-balancing is active.group-labels(per upstream)Algorithm
order, then the default group (unlabeled upstreams) last.groups (guaranteed by the strategy's accumulating
selectedUpstreamsset, which persistsacross failsafe retries/hedges within a request).
pass-on-error: false(default): on a retryable error, retry within the current group(next-best untried upstream); advance to the next group only once the current group has no
selectable upstream left.
pass-on-error: true: on a retryable error, jump straight to the next group, skippinguntried upstreams in the current group.
rate-limited) always falls through to the next group, regardless of
pass-on-error.Implementation
The feature reuses the existing selection machinery rather than duplicating it:
LabelGroupStrategy(internal/upstreams/flow/label_group_strategy.go) — a newUpstreamStrategy. A single instance lives for one request; itsSelectUpstreamwalks theordered groups with a cursor and delegates within-group selection to the existing
filterUpstreams(status/method/ws matchers, rate limits, already-tried skipping). Thefalse/truepass-on-errormodes are the only difference in how the cursor advances.NewLabelGroupStrategybuilds the ordered groups itself from the rating registry(
registry.GetSortedUpstreams→PartitionLabelGroups), mirroring howNewRatingStrategypulls its sorted list from the registry.
createStrategystays a one-liner. ANewLabelGroupStrategyWithGroupsconstructor keeps the explicit-groups path for tests.createStrategy(internal/upstreams/flow/execution_flow.go) builds aLabelGroupStrategywhen label-balancing is configured for the chain, otherwise keepsRatingStrategy. Quorum and subscribe paths are unaffected; sticky-send composes via theexisting additional matcher.
internal/config) —LabelBalancingConfig,group-labelson upstreams, theglobal + per-chain fields, a
LabelBalancingFor(chain)resolver, validation, and aninclude-defaultdefault oftrue.Upstream.GetGroupLabels()returns amapset.Set[string]built once atconstruction, so partitioning does O(1) membership checks instead of re-scanning a slice.
Tests
internal/upstreams/flow/label_group_strategy_test.go— group exhaustion before advancing,dead-group fall-through,
pass-on-errorgroup-jumping, selected-once across overlappingmembership, empty-groups, and
PartitionLabelGroupspartitioning (incl. default group).internal/config/label_balancing_test.go— validation (empty/duplicateorder),include-defaultdefaulting, andLabelBalancingForper-chain/global resolution.Docs
docs/nodecore/05-upstream-config.mdgains alabel-balancingsection plusgroup-labelsand per-chain
label-balancingfield entries.Notes
pass-on-error: trueand only a single group configured, a retryable error hasnowhere to escalate, so only one upstream is tried per request before failing — the literal
"jump on error" semantics. This is an unusual config.