Skip to content

[router] Add weight-aware Helix group selection to balance multi-key RCU - #3003

Open
pthirun wants to merge 2 commits into
linkedin:mainfrom
pthirun:pthirunavukkarasu/weight-aware-helix-group-selection
Open

[router] Add weight-aware Helix group selection to balance multi-key RCU#3003
pthirun wants to merge 2 commits into
linkedin:mainfrom
pthirun:pthirunavukkarasu/weight-aware-helix-group-selection

Conversation

@pthirun

@pthirun pthirun commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Problem Statement

Under Helix-assisted routing, the router's least-loaded group selector (HelixGroupLeastLoadedStrategy) increments a group's in-flight counter by exactly 1 per request in selectGroup, regardless of how many keys the request carries. A 1-key single-get and a 500-key multi-get therefore contribute identical load to the selected group.

Server-side read-quota enforcement, in contrast, charges per key (RCU). The result is a two-sided accounting mismatch: a group that happens to receive a run of large multi-key requests accumulates far more read-capacity load than its request count suggests, so its per-node token buckets drain and requests get throttled with HTTP 429 — even when the store-wide read quota still has substantial headroom and other groups are idle. This is more likely on stores with highly variable multi-get batch sizes.

Solution

Make the group's load counter weightable by the request's key count (an estimate of its RCU cost), so variable-size multi-key requests are balanced across Helix groups by keys/RCU rather than by raw request count.

  • HelixGroupSelectionStrategy: add selectGroup(requestId, groupCount, weight). The existing 2-arg selectGroup(requestId, groupCount) becomes a default method that delegates with weight = 1, preserving all existing callers and behavior.
  • HelixGroupLeastLoadedStrategy: counters[] becomes a weighted sum (+= weight on select, -= stored weight on finish/timeout). The per-request weight is stored alongside the group id and leak-guard timeout future in a small RequestGroupAssignment holder (replacing the previous Pair), so the decrement on completion/timeout is exactly symmetric and the counter cannot leak. Weight is clamped to >= 1 so a burst of zero/negative-weight requests cannot all pile onto one group.
  • HelixGroupRoundRobinStrategy / HelixGroupSelector: implement the new 3-arg overload (round-robin ignores weight; the selector passes it through to the underlying strategy).
  • VeniceDelegateMode: pass venicePath.getPartitionKeys().size() as the weight when the new config is enabled, else 1.

finishRequest keeps its signature (the weight is looked up from the strategy's own state), so VeniceResponseAggregator needs no change.

The existing group_pending_request metric now reflects weighted load when the feature is enabled, since counters[] is a weighted sum — giving per-group keys/RCU-in-flight visibility for free.

Trade-off: when enabled, counters[] tracks weighted (key-count) load rather than request count. Selection cost is unchanged (still a single scan under the existing synchronized block).

Code changes

  • Added new code behind a config:
    • router.helix.assisted.routing.group.selection.weight.aware.enabled — default false (no behavior change unless explicitly enabled).
  • Introduced new log lines. (None added.)

Concurrency-Specific Checks

  • Code has no race conditions or thread safety issues — all counter reads/writes remain inside the pre-existing synchronized (this) block; the new per-request weight is stored and read under the same lock.
  • Proper synchronization mechanisms are used where needed (reused the existing monitor; no new locks).
  • No blocking calls inside critical sections.
  • Verified thread-safe collections — no collection semantics changed; requestTimeoutFutureMap access remains guarded by the same lock as before.
  • Validated proper exception handling — negative-counter and duplicate-select guards are preserved.

How was this PR tested?

  • New unit tests added to TestHelixGroupLeastLoadedStrategy:
    • heavy (high-weight) request steers subsequent lighter requests away from its group,
    • weighted decrement symmetry (a heavy request fully releases its load on finish),
    • non-positive-weight clamp boundary (weight 0 still counts as 1),
    • default 2-arg overload is equivalent to explicit weight = 1 (back-compat).
  • Modified or extended existing tests (existing least-loaded, round-robin, and TestVeniceDelegateMode suites pass unchanged).
  • Verified backward compatibility — feature is config-gated and default-off; with a mocked config, isHelixGroupSelectionWeightAwareEnabled() returns false, so weight is always 1 (identical to today).
  • Local code review completed.

Compiled and tested with JDK 17 (:services:venice-router and :internal:venice-common).

Does this PR introduce any user-facing or breaking changes?

  • No. The behavior is gated behind a new config that defaults to false; when disabled, group selection is byte-for-byte identical to the current implementation.

🤖 Generated with GitHub Copilot CLI

pthirun and others added 2 commits September 1, 2026 12:04
The Helix-assisted-routing least-loaded group selector counts every request
as +1 when picking a group, regardless of key count. A 1-key request and a
500-key multi-get therefore weigh identically, so a group can absorb
disproportionate read-capacity (RCU) and get throttled with HTTP 429 while
other groups retain capacity and the store-wide read quota still has headroom.

Make the group's load counter weightable by the request's key count (an
estimate of its RCU cost) so variable-size multi-key requests are balanced by
keys/RCU rather than by raw request count.

- HelixGroupSelectionStrategy: add selectGroup(requestId, groupCount, weight);
  the existing 2-arg overload becomes a default delegating with weight=1.
- HelixGroupLeastLoadedStrategy: counters[] becomes a weighted sum
  (+= weight on select, -= stored weight on finish/timeout). The per-request
  weight is stored in a RequestGroupAssignment holder so the decrement is
  symmetric; weight is clamped to >= 1.
- HelixGroupRoundRobinStrategy / HelixGroupSelector: implement the 3-arg
  overload (round-robin ignores weight; selector passes it through).
- VeniceDelegateMode: pass venicePath.getPartitionKeys().size() as the weight
  when the new config is enabled, else 1.
- New config
router.helix.assisted.routing.group.selection.weight.aware.enabled
  (default false) gates the behavior, so it is a no-op unless explicitly
enabled.

finishRequest keeps its signature (the weight is stored in the strategy), so
no change is needed in VeniceResponseAggregator.

Added tests for heavy-request steering, weighted decrement symmetry, the
non-positive-weight clamp (boundary), and default-overload back-compat.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant