Handle waking up a closed selector in Reactor#add #3005
Merged
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
JuanitoFatas
force-pushed
the
reactor-closed-selector
branch
from
e7e4b4c to
99b58b8
Compare
Member
|
@JuanitoFatas Thanks for looking at this. I've also seen very intermittent errors in CI, and it seems to be a problem related to how 'lost/dropped' connections are handled in the Reactor. Does that sound like it's similar to your issue? Haven't had time to look at it, as I wanted to look at how a 'lost/dropped' connection is removed... EDIT: Forget the above, the error is raised in NIO code, and 'selector is closed' is the message. |
Member
|
Just a quick look, should this be: # Returns false if the reactor is already shut down.
def add(client)
@input << client
@selector.wakeup
true
rescue ClosedQueueError, IOError
false
end |
nateberkopec
added
waiting-for-changes
Co-Authored-By: MSP-Greg <[email protected]>
JuanitoFatas
force-pushed
the
reactor-closed-selector
branch
from
99b58b8 to
1a2db33
Compare
Member
|
Test fails are not related, also happening on main. |
MSP-Greg
added
bug
and removed
waiting-for-changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 Pull Request rescues
IOErrorwhen we try to wake up a closed selector.I noticed there are
IOErrors from this code path (see below). It may be auto-scaling scale up/down an instance. It does not seems to relate to during deployment / restart. I am not really sure. It only happens 4-5 times for the past 30 days in an app (with reasonable, constant traffic) I am working on.Seems like similar to
#shutdownmethod, we should handle waking up a closed selector.I am not sure how to add a test for this.
If needs, please let me know how could I add some instrument to the app I’m working on, so I could help report more details for helping with improving this area. Thanks!