Skip to content

Conversation

@starius
Copy link
Collaborator

@starius starius commented Nov 12, 2025

Started from CI failure of TestSweepBatcherHandleSweepRace/loopdb that surfaced ErrBatcherShuttingDown when the AddSweep loop caught a batch right as it finished. Reproduced via:

go test ./sweepbatcher -run TestSweepBatcherHandleSweepRace/loopdb -cpu=12,4,8 -count=1

Discovered that handleSweeps treated ErrBatchShuttingDown from batch.addSweeps as fatal, so the batcher exited even though the sweep was already confirmed. Fixed by treating that error as "batch already done" so we fall through to the persisted status/monitorSpend path, and added a regression test that
deterministically simulates the shutdown window.

New regression test added:

go test ./sweepbatcher -run TestSweepBatcherHandleBatchShutdown

Pull Request Checklist

  • Update release_notes.md if your PR contains major features, breaking changes or bugfixes

Started from CI failure of TestSweepBatcherHandleSweepRace/loopdb [1] that
surfaced ErrBatcherShuttingDown when the AddSweep loop caught a batch right as
it finished. Reproduced via:

go test ./sweepbatcher -run TestSweepBatcherHandleSweepRace/loopdb -cpu=12,4,8 -count=1

Discovered that handleSweeps treated ErrBatchShuttingDown from batch.addSweeps
as fatal, so the batcher exited even though the sweep was already confirmed.
Fixed by treating that error as "batch already done" so we fall through to the
persisted status/monitorSpend path, and added a regression test that
deterministically simulates the shutdown window.

New regression test added:
go test ./sweepbatcher -run TestSweepBatcherHandleBatchShutdown

[1] https://github.com/lightninglabs/loop/actions/runs/19282552307/job/55136597881?pr=1041
@starius starius marked this pull request as ready for review November 12, 2025 05:49
Copy link
Member

@bhandras bhandras left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! 🎉

Copy link
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

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

I have one question, other than that looks good to me!

if batch.sweepExists(sweep.outpoint) {
accepted, err := batch.addSweeps(ctx, sweeps)
if err != nil && !errors.Is(err, ErrBatchShuttingDown) {
if errors.Is(err, ErrBatchShuttingDown) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIUC there is no way that addSweeps still adds the sweep and returns ErrBatchShuttingDown, right?

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.

3 participants