signal processPodEvent routine to close PodEvent channel #1888
Conversation
This file contains hidden or 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
FxKu
requested review from
CyberDem0n,
Jan-M,
idanovinda,
jopadi and
sdudoladov
as code owners
dmvolod
reviewed
May 13, 2022
| stopCh := make(chan struct{}) | ||
| ch := c.registerUnbufferedPodSubscriber(podName) | ||
| // send special end event to trigger removal of PodEvent channel by processPodEvent | ||
| defer c.ReceivePodEvent(PodEvent{PodName: types.NamespacedName(podName), EventType: PodEventEnd}) |
FxKu
force-pushed
the
switchover-subscriber-fix-2
branch
from
0b38d9b to
7951e9c
Compare
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.
This is a continuation of #1876 that already improved on the situation that operator runs into a panic during switchover. However, there's still no guarantee that
Switchovercloses the PodEvent channel when callingunregisterPodSubscriberwhile theprocessPodEventroutine still sends something.The idea of this PR is to send a new PodEvent type to the
processPodEventroutine and make it responsible for closing the channel. Unfortunately, the race condition remains the same, but instead of a panic we will face a deadlock. Switchover method can send the special END event which will be added to processPodEventQueue, but there might still be more events in the queue that will then be send to the channel. However, the consumer (in this case waitForPodLabel) is already finished. Now the go routine is blocked and will no proceed.One solution could be to define a buffered channel when we do the switchover. Buffered channels do not block go routines when events sent to a channel are not consumed. However, using the correct size for a channel might be a challenge, since it's not known how long the actual switchover will take and how many events are really sent during one second. And when the channel is full we have a deadlock again.