Kill stalled tier2 segments instead of slow ones - #860
Merged
Conversation
Tier2 aborted a segment once it exceeded a fixed 60 minute budget. That is fatal to workloads that are expensive but healthy: a segment making thousands of eth_call per block can take slightly longer than the budget while still advancing block by block, get killed, and -- since a killed segment is never cached -- have its retry redo the same work and hit the same wall. Such a request never completes, no matter how many times the client reconnects. Replace the fixed budget with a stall timeout (10 minutes, resets on every block processed) and demote segmentExecutionTimeout to an absolute backstop, raising its default from 60 minutes to 4 hours. A wedged segment is still killed promptly, and because a single block is already bounded by blockExecutionTimeout (3 minutes), the stall timeout cannot be tripped by one legitimately slow block. The decision is extracted into segmentWatchdog so it can be tested against wall-clock scenarios instead of only inside the ProcessRange goroutine. Stats.processedBlocks becomes atomic: it is written by the block-processing goroutine and read by the watchdog goroutine. That read already existed for the slow-query log, but control flow now depends on it. Also fix a tier1 shutdown during parallel backprocessing being reported to the client as Internal rather than Unavailable. The "endpoint is shutting down, please reconnect" error is wrapped several times on its way up from the scheduler, and was matched with a pointer comparison, so it never took the Unavailable path and clients lost the reconnect signal during a rollout.
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
A customer's
map_token_balancessubstreams on eth-mainnet was reconnecting in a loop for hours while delivering ~0 blocks. Every attempt died with:Traced end to end:
service/tier2.gowatchdog fires once a segment exceedssegmentExecutionTimeout(60min), cancelling withErrRequestActiveForTooLong.codes.DeadlineExceededto aRetryableErr.orchestrator/work/worker.go:161intercepts first:streamOutput && upstream.DataSent()makes it fatal with zero retries — correctly so, since those blocks already reached the client and retrying would duplicate them.The module makes roughly a thousand
eth_callper block (therpc:eth_callmetric counts batches, not individual calls). Segments were landing right at the edge of the 60 minute budget. A segment that misses is never cached, so the retry redoes all of it and races the same clock again — the request could never complete regardless of how many times the client reconnected.Change
Tier2 now aborts a segment when it stops making progress rather than when it merely takes a long time.
WithSegmentStallTimeout), reset on every block processed.segmentExecutionTimeoutdemoted to an absolute backstop, default raised 60min → 4h.segmentWatchdog(service/segment_watchdog.go) so it is testable against wall-clock scenarios rather than only inside theProcessRangegoroutine.A wedged segment is still killed promptly. Because a single block is already bounded by
blockExecutionTimeout(3min), the stall timeout cannot be tripped by one legitimately slow block.Observability:
request active for a long timegained asince_last_progressfield, and a stall-kill reportsrequest stalled, no block progress, distinct fromrequest active for too long.Also in here
Two smaller fixes found while tracing:
Stats.processedBlocksis now atomic. It is written by the block-processing goroutine and read by the watchdog goroutine. That cross-goroutine read already existed for the slow-query log, but control flow now depends on it.Internalinstead ofUnavailable. Theendpoint is shutting down, please reconnecterror is wrapped several times on its way up from the scheduler and was matched with a pointer comparison (err == errShuttingDown), so it never took theUnavailablepath. Clients lost the reconnect signal during every tier1 rollout.Testing
service/segment_watchdog_test.go— covers the customer scenario (one block per 5min for 3h survives, where the old budget killed it at 60min), stall detection, deadline reset on progress, the absolute backstop, precedence, and disabled timeouts.service/error_shutting_down_test.go— assertsUnavailablefor both the bare and the four-deep-wrapped shutdown error.go build ./...clean;service,metrics,orchestrator,pipeline,apppass;-raceclean onservice,metrics,pipeline.Reviewer notes
worker.go:161is deliberately untouched. This change avoids tripping the no-retry path rather than softening it.eth_call/block remains the root cause on the module side; caching token metadata in a store would be the real fix there.