FAB-18291 Ch.Part.API: BlockPuller that can be stopped, improve follower Halt() responsiveness #2050
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tock-ibm
changed the title
wlahti
reviewed
Oct 27, 2020
wlahti
previously approved these changes
Oct 27, 2020
yacovm
reviewed
Oct 28, 2020
| // A 'stopper' goroutine may signal the go-routine servicing PullBlock & HeightsByEndpoints to stop by closing this | ||
| // channel. Note: all methods of the BlockPuller must be serviced by a single goroutine, it is not thread safe. | ||
| // It is the responsibility of the 'stopper' not to close the channel more then once. | ||
| StopChannel chan struct{} |
There was a problem hiding this comment.
The way of externally closing an internal channel is somewhat crude. I think that we should maintain the channel ourselves from within this struct, that is, to close it in a "Close" instance method.
There was a problem hiding this comment.
It is not necessarily an internal channel. It is possible to give the BlockPuller an external channel, in the case of the follower, the channel that signals the follower itself to stop. This way that channel is closed when someone calls follower.Halt() and it signals both the follower and the block puller it holds to stop retrying and exit. This is fine because in the follower we have only a single goroutine that goes in and out of the block puller.
yacovm
reviewed
Oct 28, 2020
yacovm
reviewed
Oct 29, 2020
When a follower is started with a block that has bad endpoints, it will cause the internal BlockPuller to retry a lot of times. Calling Halt() on said follower will block until the BlockPuller gives up, which can take a very long time. This causes an attempt to Remove that channel via the participation API to block for too long. This commit improves the BlockPuller by adding a channel to signal it to stop retrying and exit. It also integrates with the follower.Chain to make it more resposive to Halt(). Signed-off-by: Yoav Tock <tock@il.ibm.com> Change-Id: I49354d94a28402ce69a469689c12ea595b3e0a7e
yacovm
approved these changes
Oct 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a follower is started with a block that has bad endpoints,
it will cause the internal BlockPuller to retry a lot of times.
Calling Halt() on said follower will block until the BlockPuller
gives up, which can take a very long time. This causes an attempt
to Remove that channel via the participation API to block for too long.
This commit improves the BlockPuller by adding a channel to signal
it to stop retrying and exit. It also integrates with the
follower.Chain to make it more responsive to Halt().
Signed-off-by: Yoav Tock tock@il.ibm.com
Change-Id: I49354d94a28402ce69a469689c12ea595b3e0a7e
Type of change
Related issues
FAB-18291