Health-aware failover for Hive-Engine nodes#56
Conversation
Engine data intermittently blanked in portfolio responses: the ported node selection kept a sticky randomly-picked node with one blind random retry, and the pool contained a node that no longer resolves plus two that fail intermittently. When the pick landed badly twice, the engine fetchers threw and the portfolio compiled without engine tokens. - Extract HiveRpcClient's per-node health bookkeeping into a shared NodeHealthTracker (no behavior change; existing failover tests cover it). - Add EngineRpcClient on the same tracker: find queries require a result array (an error payload from a fixed-shape find is a node failure, not an application error) and walk the pool best-first; the engine-api passthrough fails over on transport errors and 429/5xx only, capped at three attempts. - Refresh the node pool from the public health meter: drop the dead node and two 0% nodes, add four healthy ones; order by observed reliability. - Per-attempt find timeout of 2s so failover fits inside the portfolioV2 engine leg budget instead of one slow node consuming all of it.
Greptile SummaryThis PR adds health-aware failover for Hive-Engine requests. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix: pool the history nodes behind the s..." | Re-trigger Greptile |
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe change centralizes upstream node health tracking, refactors Hive RPC to use it, adds health-aware Hive-Engine failover, routes wallet engine requests through the new client, and adds loopback tests and documentation. ChangesRPC failover
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WalletApi
participant EngineRpcClient
participant NodeHealthTracker
participant EngineNode
WalletApi->>EngineRpcClient: Send Find or ContractsRaw request
EngineRpcClient->>NodeHealthTracker: Get ordered node indices
EngineRpcClient->>EngineNode: POST /contracts
EngineNode-->>EngineRpcClient: Return result or failure response
EngineRpcClient->>NodeHealthTracker: Record node outcome
EngineRpcClient-->>WalletApi: Return result or upstream response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dotnet/EcencyApi.Tests/EngineFailoverTests.cs`:
- Around line 187-212: Add a ContractsRaw test covering the separate 429
handling path: configure one rate-limited stub and one healthy stub, make two
sequential ContractsRaw calls, assert both calls succeed, and verify the
rate-limited node has one hit while the healthy node has two. Follow the
existing test setup and assertion patterns in ContractsRaw failover tests.
In `@dotnet/EcencyApi/Handlers/WalletApi.Engine.cs`:
- Around line 28-41: Update the EngineClient initialization to load the ordered
RPC node pool from deployment configuration instead of embedding hostnames in
source. Remove the infrastructure-specific reliability comment and hard-coded
endpoint list, while preserving runtime latency-based reordering and
configuration order as the cold-start tiebreak.
In `@dotnet/EcencyApi/Infrastructure/NodeHealthTracker.cs`:
- Around line 62-69: Remove the RateLimitStreak reset from RecordSuccess(int
nodeIndex, double elapsedMs), while preserving the ConsecutiveFailures reset and
latency recording. Let RecordRateLimited remain responsible for the documented
time-based throttle reset after 120 seconds without a throttle.
- Around line 159-167: Update ParseRetryAfterMs so converting nonnegative
Retry-After seconds to milliseconds cannot overflow int; validate the seconds
value against the maximum representable millisecond duration before multiplying,
and return null for values that exceed that limit while preserving existing
handling of invalid or negative headers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eaf55164-8194-4699-bd62-0db56a6b728a
📒 Files selected for processing (6)
CLAUDE.mddotnet/EcencyApi.Tests/EngineFailoverTests.csdotnet/EcencyApi/Handlers/WalletApi.Engine.csdotnet/EcencyApi/Infrastructure/EngineRpcClient.csdotnet/EcencyApi/Infrastructure/HiveRpcClient.csdotnet/EcencyApi/Infrastructure/NodeHealthTracker.cs
The account-history passthrough proxied a single hardcoded host with no retry. Generalize EngineRpcClient's passthrough loop to any method+path and give the history route a two-node pool; per-attempt timeout halved so the two-attempt envelope matches the original 30s. Review fixes: clamp huge Retry-After delta-seconds instead of overflowing into a negative park window; add ContractsRaw 429-parking coverage.
1 participant
Engine token data intermittently blanked in portfolio responses. The engine node selection kept a sticky randomly-picked node with one blind random retry, and the pool contained a node that no longer resolves in DNS plus two that fail intermittently — when the pick landed badly twice, the engine fetchers threw and the portfolio compiled without engine tokens.
Changes
HiveRpcClient's per-node health bookkeeping (429 parking, recent-failure deprioritization, latency EWMA ordering) into a sharedNodeHealthTracker. No behavior change on the Hive side; the existingHiveRpcFailoverTestscover the refactor.EngineRpcClienton the same tracker:Find(portfolio balances/tokens/metrics): these are fixed-shape queries that always yield aresultarray on a healthy node, so an error payload or non-JSON body is treated as a node failure and rolls over to the next node, walking the pool best-first. Per-attempt timeout is 2s so failover fits inside the portfolioV2 engine leg budget instead of one slow node consuming all of it.ContractsRaw(engine-apipassthrough): fails over only on transport errors and 429/5xx — any other response belongs to the caller's query and pipes as-is. Attempts capped at 3.Testing
EngineFailoverTestsagainst stub HTTP nodes: dead node, error-payload node, 429 parking, failed-node deprioritization, all-nodes-failing, passthrough failover/as-is/last-response/attempt-cap semantics. Full suite: 52 passed.portfolio-v2returns the full engine layer (60 tokens for a token-heavy account) consistently across repeated calls;engine-apipassthrough verified.Summary by CodeRabbit
New Features
Bug Fixes
Tests