Skip to content

op-batcher: exit process on criticial throttling RPC error#17924

Merged
geoknee merged 19 commits intodevelopfrom
gk/batcher-crit
Dec 9, 2025
Merged

op-batcher: exit process on criticial throttling RPC error#17924
geoknee merged 19 commits intodevelopfrom
gk/batcher-crit

Conversation

@geoknee
Copy link
Copy Markdown
Contributor

@geoknee geoknee commented Oct 18, 2025

Confirmed manually that with these changes, the process is exited instead of just stopping the "work" of the batcher only.

Closes #17768

@geoknee geoknee marked this pull request as ready for review October 18, 2025 21:57
@geoknee geoknee requested review from a team and scharissis October 18, 2025 21:57
Comment thread op-batcher/batcher/driver.go
Comment thread op-batcher/batcher/driver.go
Comment thread op-batcher/batcher/driver.go
@geoknee geoknee requested a review from sebastianst October 30, 2025 18:22
Copy link
Copy Markdown
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's an easier way (that actually also stops the batcher in tests).

Comment thread op-batcher/batcher/driver.go Outdated
Comment thread op-batcher/batcher/driver.go Outdated
Comment thread op-batcher/batcher/driver.go
Comment thread op-devstack/sysgo/l2_batcher.go Outdated
Comment thread op-batcher/batcher/service.go Outdated
Comment thread op-batcher/batcher/batch_submitter.go
@opgitgovernance opgitgovernance added the S-stale Status: Will be closed unless there is activity label Nov 14, 2025
@opgitgovernance
Copy link
Copy Markdown
Contributor

This pr has been automatically marked as stale and will be closed in 5 days if no updates

@geoknee geoknee removed the S-stale Status: Will be closed unless there is activity label Nov 19, 2025
Store closeApp in BatcherService and propagate it through DriverSetup so
the driver can access it without a separate parameter. Update
BatcherServiceFromCLIConfig/initFromCLIConfig signatures (closeApp moved
earlier), remove closeApp arg from NewBatchSubmitter, and guard
shutdownOnCriticalError against nil. Update call sites accordingly.
Use context.WithCancelCause and pass its cancel function when creating
the batcher so the service can be stopped with a cancel cause. Also
expose closeApp in DriverSetup and simplify a test by importing slices
and replacing manual loops with slices.Contains. Propagate cancel cause
to BatcherService
Comment thread op-batcher/batcher/driver.go
geoknee and others added 2 commits December 4, 2025 15:40
Co-authored-by: almanax-ai[bot] <174396398+almanax-ai[bot]@users.noreply.github.com>
Replace context.WithCancelCause with context.WithCancel and add a
closeAppFn that calls p.FailNow with the cause and cancels the batcher
context. Pass closeAppFn into BatcherServiceFromCLIConfig and import fmt
so tests fail immediately on critical batcher errors. Use cancelable
context and failure hook for batcher

Log the failure cause and cancel the batcher context on critical errors
Comment thread op-batcher/batcher/batch_submitter.go
Comment thread op-batcher/batcher/driver.go
@geoknee geoknee enabled auto-merge December 9, 2025 15:07
@geoknee geoknee added this pull request to the merge queue Dec 9, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Dec 9, 2025
@geoknee geoknee added this pull request to the merge queue Dec 9, 2025
Merged via the queue into develop with commit b7d48f0 Dec 9, 2025
83 checks passed
@geoknee geoknee deleted the gk/batcher-crit branch December 9, 2025 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-op-batcher Area: op-batcher

Projects

None yet

Development

Successfully merging this pull request may close these issues.

op-batcher: ensure full shutdown when "SetMaxDASize RPC method unavailable"

4 participants