Skip to content

core/bloombits: use single channel for shutdown#20878

Merged
fjl merged 5 commits into
ethereum:masterfrom
ucwong:bloombits
Jul 29, 2020
Merged

core/bloombits: use single channel for shutdown#20878
fjl merged 5 commits into
ethereum:masterfrom
ucwong:bloombits

Conversation

@ucwong
Copy link
Copy Markdown
Contributor

@ucwong ucwong commented Apr 3, 2020

No description provided.

holiman
holiman approved these changes Apr 7, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

Looks like it makes sense to me, what do you say @rjl493456442 ?

rjl493456442
rjl493456442 approved these changes Apr 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

LGTM

@fjl
Copy link
Copy Markdown
Contributor

fjl commented Apr 7, 2020 •
edited
Loading

I think it would be better to remove the kill channel instead. It would be nice to know why bloombits requires a two-stage shutdown. We can usually make do with just one quit channel.

@fjl fjl changed the title core/bloombits/matcher : Call stop timer if return before timeout core/bloombits: call stop timer if return before timeout Apr 7, 2020
fjl
fjl reviewed Apr 7, 2020
View reviewed changes
Comment thread core/bloombits/matcher.go Outdated
fjl
fjl reviewed Apr 9, 2020
View reviewed changes
Comment thread core/bloombits/matcher.go
@ucwong ucwong requested a review from fjl April 22, 2020 01:40
@ucwong ucwong changed the title core/bloombits: call stop timer if return before timeout core/bloombits: remove kill channel for shutting down, using quit channel only Apr 28, 2020
@fjl fjl changed the title core/bloombits: remove kill channel for shutting down, using quit channel only core/bloombits: use single channel for shutdown Jul 28, 2020
@fjl fjl merged commit 9e04c5e into ethereum:master Jul 29, 2020
This was referenced Feb 24, 2021
Closed
Merged
enriquefynn pushed a commit to enriquefynn/go-ethereum that referenced this pull request Mar 10, 2021
@ucwong @fjl @enriquefynn
core/bloombits: use single channel for shutdown (ethereum#20878)
f751060 
This replaces the two-stage shutdown scheme with the one we
use almost everywhere else: a single quit channel signalling
termination.

Co-authored-by: Felix Lange <fjl@twurst.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjl fjl fjl approved these changes

@rjl493456442 rjl493456442 rjl493456442 approved these changes

@karalabe karalabe Awaiting requested review from karalabe

+1 more reviewer

@holiman holiman holiman approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@fjl fjl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ucwong @fjl @holiman @rjl493456442 @adamschmideg