Skip to content

Fix retention hanging forever when the permanent store is disabled - #59

Open
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/retention-permanent-store-channel-leak
Open

Fix retention hanging forever when the permanent store is disabled#59
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/retention-permanent-store-channel-leak

Conversation

@damilolaedwards

Copy link
Copy Markdown

Summary

purgeOldBeaconBlocks queues every aged out block with the permanent store and then waits on a channel that the permanent store only closes once the block is actually handed off for processing. When the permanent store is disabled, which is the default, or stopped, or its queue is full, QueueBlock returned without ever touching that channel, so the wait never completed.

Since all six retention purges run one after another on a single goroutine, this stalls cleanup for every data type the moment the first beacon block ages out, silently and permanently. No attacker or unusual configuration is needed, just the documented default.

QueueBlock now closes the channel on every path, not only the one where a block is actually queued. Retention's wait also now selects on context cancellation as a second line of defense, so a purge in progress during shutdown cannot block indefinitely either.

Changes

  • pkg/server/service/indexer/permanent_store.go: QueueBlock closes ProcessedChan on every early return path via a small shared helper, also used by processBlock's existing close
  • pkg/server/service/indexer/retention.go: the wait on ProcessedChan now also selects on ctx.Done()
  • pkg/server/service/indexer/retention_test.go: two new tests, no prior test coverage existed for retention at all

Test plan

  • go build ./... and go vet ./... pass
  • New tests pass with the fix applied
  • Confirmed the tests actually catch the bug: reverted the fix, reran them, watched both fail with a timeout, then restored the fix and confirmed they pass again
  • Pre-existing Docker-dependent tests elsewhere in this package could not be run in this environment (no Docker available), unrelated to this change

purgeOldBeaconBlocks queued every aged out block with the permanent
store and then waited on a channel that the permanent store only
closed once the block was actually handed off for processing. When
the permanent store is disabled, which is the default, or stopped,
or its queue is full, QueueBlock returned without ever touching that
channel, so the wait never completed. Since all retention purges run
one after another on a single goroutine, this stalled cleanup for
every data type the first time a beacon block aged out, silently and
permanently.

QueueBlock now closes the channel on every path, not only the one
where a block is actually queued. Retention's wait also now selects
on context cancellation as a second line of defense, so a purge in
progress during shutdown does not block indefinitely either.
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