fix(server): back off, retry, and de-race the EVM indexer fetch loop#311
Conversation
puneet2019
force-pushed
the
refactor/adopt-cosmos-evm-indexer-service
branch
from
168ce9f to
ac48db7
Compare
puneet2019
force-pushed
the
refactor/adopt-cosmos-evm-indexer-service
branch
from
ac48db7 to
ef7dbf0
Compare
phenix3443
left a comment
phenix3443
left a comment
There was a problem hiding this comment.
I found one blocking issue.
Adopting cosmos/evm's EVMIndexerService in the pinned upstream version introduces a stall-once-failed behavior: after the first transient Block / BlockResults fetch error, the latched blockErr prevents the service from ever retrying indexing work, so the EVM tx indexer can stop progressing until the node restarts. I left the detail inline on server/start.go where the upstream service is wired in.
| // cosmos/evm's indexer service (identical ctor; upstream also waits | ||
| // before retrying when a Block/BlockResults fetch errors, instead of | ||
| // busy-looping the failing fetch). | ||
| indexerService := cosmosevmserver.NewEVMIndexerService(idxer, clientCtx.Client.(rpcclient.Client)) |
There was a problem hiding this comment.
Switching to cosmos/evm's NewEVMIndexerService pulls in a regression from the pinned upstream v0.6.0: in server/indexer_service.go, blockErr is set on a transient Block / BlockResults fetch failure and then used as a permanent gate (if latestBlock <= lastBlock || blockErr != nil), but it is never cleared before retry. After the first fetch error, the service only sleeps and never re-enters the indexing loop, so EVM tx indexing can stall until process restart. The previous in-tree copy busy-looped, which was ugly but at least self-recovered once the storage race cleared.
This PR set out to adopt cosmos/evm's EVMIndexerService for its wait-before-retry on Block/BlockResults fetch errors; review (phenix) caught that upstream's version (still on their main) latches the fetch error and never clears it — after the first transient failure the loop only sleeps and indexing stalls until restart. The old in-tree copy had the opposite defect: it busy-looped the failing fetch with no backoff. Writing the regression test also surfaced a third defect present in both: the latest-height variable is written by the new-block subscription goroutine and read by the indexing loop unsynchronized (go test -race fails on both prior versions). Keep the service in-tree on upstream's structure with all three fixed: back off on fetch errors (upstream's improvement), clear the error before retrying (the missing line), and make latestBlock an atomic. The regression test drives the loop with a scripted RPC client and fails on each historical defect: it times out on the latched-stall version, exceeds the bounded fetch-attempt count on the busy-loop version, and trips -race on the unsynchronized version. Signed-off-by: puneetmahajan <59960662+puneet2019@users.noreply.github.com>
puneet2019
force-pushed
the
refactor/adopt-cosmos-evm-indexer-service
branch
from
ef7dbf0 to
66c5e48
Compare
puneet2019
changed the title
2 participants
The three defects (all in the same ~40-line loop)
Block/BlockResultsfetch with no backoff (CPU burn; can occur from drive latency between a block existing and its results being saved)mainblockErrgates the loop but is never cleared — after the first transient fetch error the service only sleeps; indexing stops until restartgo test -racefails on both prior versions (surfaced while writing the regression test)Fix (
server/indexer_service.go, +39/−13 vs main)Upstream's structure + backoff, plus
blockErr = nilafter the wait (so the loop actually retries), pluslatestBlockas anatomic.Int64.Regression test (
server/indexer_service_test.go)Mock RPC client scripts a transient
BlockResultsfailure (fails twice, then succeeds); new-block signals drive the loop with no real-time waits. Verified to fail on each historical defect:indexed so far: map[] attempts=1)-racefailureand passes on the fixed code (
go test ./server/... -raceclean).Upstream
The latch + race are live in cosmos/evm
main. Happy to send the completing fix upstream (same pattern as cosmos/evm#1214); until then the in-tree copy is the correct choice — adopting upstream would trade a busy-loop for a stall.🤖 Generated with Claude Code