Skip to content

Cancel sidecar parent-block fetches on sync service shutdown - #17462

Open
syjn99 wants to merge 4 commits into
developfrom
fix/ctx-parent-block-fetches
Open

Cancel sidecar parent-block fetches on sync service shutdown#17462
syjn99 wants to merge 4 commits into
developfrom
fix/ctx-parent-block-fetches

Conversation

@syjn99

@syjn99 syjn99 commented Sep 7, 2026

Copy link
Copy Markdown
Member

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?

When a blob or data column sidecar arrives over gossip before its parent block, the (blob/data column) validator spawns a goroutine that requests the parent by root (sendBatchRootRequest).
Since #13061 that goroutine ran on context.Background(), so it never observed beacon node shutdown, and a cancelled context was not honored mid-request either.

This PR:

  • Runs the sidecar parent fetch on the sync service context (s.ctx) instead of context.Background().
  • Registers context.AfterFunc(ctx, stream.Reset) in SendBeaconBlocksByRootRequest and SendExecutionPayloadEnvelopesByRootRequest, so cancellation interrupts a blocked response read instead of waiting for the next chunk deadline.
  • Checks ctx.Err() at the top of each retry and after each attempt in sendBatchRootRequest, so a cancelled request returns immediately instead of burning the remaining numOfTries attempts.
  • Bounds the optimistic payload envelope fetch with respTimeout per attempt, the same bound sendBeaconBlocksRequest already applies to the block request. Previously the envelope request inherited the caller's context with no timeout of its own.

Which issue(s) does this PR fix?

N/A

Other notes for review

Acknowledgements

  • I have read CONTRIBUTING.md.
  • I have included a uniquely named changelog fragment file.
  • I have added a description with sufficient context for reviewers to understand this PR.
  • I have tested that my changes work as expected and I added a testing plan to the PR description (if applicable).

- #13061

This PR creates a new context and puts it in `sendBatchRootRequest`. This commit uses more correct context which is a service context, following #17057's choice as well.

This commit also registers `stream.Reset()` after the context for two by-root senders.

Unit test: `TestValidateSidecar_ParentRequestServiceLifetime`
@syjn99
syjn99 requested a review from a team as a code owner September 7, 2026 07:58
@syjn99
syjn99 marked this pull request as draft September 7, 2026 09:00
@syjn99
syjn99 marked this pull request as ready for review September 7, 2026 09:09
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