Skip to content

fix: shut down the P2P subsystem on DB.Close - #5133

Draft
mennatnaga wants to merge 1 commit into
menna/fetcher-prefix-iteratorfrom
menna/p2p-close-race
Draft

fix: shut down the P2P subsystem on DB.Close#5133
mennatnaga wants to merge 1 commit into
menna/fetcher-prefix-iteratorfrom
menna/p2p-close-race

Conversation

@mennatnaga

@mennatnaga mennatnaga commented Aug 11, 2026

Copy link
Copy Markdown
Member

Stacked on #5137.

Close closed the message queue to stop the workers. The pubsub dispatcher keeps calling the handler until libp2p tears the subscription down, which Close does not do, and a send on a closed channel is a ready case in a select rather than a fallthrough to the default arm, so a message arriving during shutdown panics the dispatcher. The queue is now left open and a separate signal ends the worker pool.

Not complete. (*P2P).Close has no production caller, so the panic is unreachable as things stand: node.Peer is github.com/sourcenetwork/go-p2p, so n.peer.Close() does not reach this one. Wiring DB.Close to shut the P2P subsystem down is what makes this matter, and that is not done here.

@mennatnaga
mennatnaga force-pushed the menna/car-import-existence-guard branch from 8796070 to 5573071 Compare August 12, 2026 14:03
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from b7b1c46 to d359c26 Compare August 12, 2026 14:03
@mennatnaga
mennatnaga force-pushed the menna/car-import-existence-guard branch from 5573071 to 3567a2f Compare August 12, 2026 15:14
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from d359c26 to 0550de8 Compare August 12, 2026 15:14
@mennatnaga mennatnaga changed the title fix: stop accepting pubsub messages without closing the queue fix: shut down the P2P subsystem on DB.Close Aug 12, 2026
@mennatnaga
mennatnaga force-pushed the menna/car-import-existence-guard branch from 3567a2f to 80e8510 Compare August 13, 2026 11:26
Base automatically changed from menna/car-import-existence-guard to menna/blockstore-orphan-gc August 13, 2026 11:26
@mennatnaga
mennatnaga changed the base branch from menna/blockstore-orphan-gc to menna/fetcher-prefix-iterator August 13, 2026 12:04
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 0550de8 to f191251 Compare August 13, 2026 12:04
@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from 7313e0b to 4d087a1 Compare August 13, 2026 16:14
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from f191251 to 29198d5 Compare August 13, 2026 16:21
Comment thread internal/db/p2p/p2p.go
Comment on lines +687 to 690
case <-p.stopAccepting:
p.releaseQueueBytes(size)
case <-p.ctx.Done():
p.releaseQueueBytes(size)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

question: Without this PR, shouldn't the ctx.Done case be able to handle this graceful shutdown if you cancel the context object referenced?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

you're right, DB.Close cancels p.ctx, so ctx.Done already covers this - I will fix this once I finalise the other prs

@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from 4d087a1 to 4f5c155 Compare August 13, 2026 23:15
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 29198d5 to 812fe3e Compare August 13, 2026 23:15
@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from 4f5c155 to 161af74 Compare August 14, 2026 00:45
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 812fe3e to 26724c7 Compare August 14, 2026 00:45
@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from 161af74 to fe2b7e7 Compare August 14, 2026 18:28
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 26724c7 to 8921989 Compare August 14, 2026 18:28
@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from fe2b7e7 to 1f4e3d1 Compare August 14, 2026 18:56
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 8921989 to 879ea9b Compare August 14, 2026 18:56
@mennatnaga
mennatnaga force-pushed the menna/fetcher-prefix-iterator branch from 1f4e3d1 to 1501d69 Compare August 27, 2026 13:23
@mennatnaga
mennatnaga force-pushed the menna/p2p-close-race branch from 879ea9b to 278962c Compare August 27, 2026 13:23
@shahzadlone shahzadlone added the shinzo Shinzo related work label Aug 27, 2026
An error occurred while trying to automatically change base from menna/fetcher-prefix-iterator to menna/purge-chunk-size August 28, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

shinzo Shinzo related work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants