Close open connections from parent after fork #16953
Merged
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
|
Output from a run without clearing the connection pool. This shows all of the commonly hit issues, incompatible marshal file, too large packet, and just the wrong message returned: |
jrafanie
reviewed
Feb 6, 2018
| # Close all open DRb connections so that connections in the parent's memory space | ||
| # which is shared due to forking the child process do not pollute the child's DRb | ||
| # connection pool. This can lead to errors when the children connect to a server | ||
| # and get an incorrect response back. |
There was a problem hiding this comment.
That's worded as well as it can be. Nice job!
jrafanie
reviewed
Feb 6, 2018
app/models/miq_worker.rb
Outdated
| # | ||
| # ref: https://bugs.ruby-lang.org/issues/2718 | ||
| def self.close_drb_pool_connections | ||
| require 'drb/drb' |
There was a problem hiding this comment.
#MAJOR HACK approaching...
It's a stable api since it's been there since at least 2003 😆 🤣
jrafanie
approved these changes
Feb 6, 2018
Maybe say " to go to the wrong drb client"` |
Great point, also @Fryguy mentioned we might want to synchronize on that mutex so I'll make both of those changes |
DRb::DRbConn keeps a global pool of open connections which is shared by child processes when they are forked from a parent. If this parent executes a DRb call prior to forking a child process the child picks up this open connection and uses it which can cause replies from the server to go to the wrong DRb client. There is a long standing ruby bug https://bugs.ruby-lang.org/issues/2718 which describes the issue and has reproducer code attached. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1385038
agrare
force-pushed
the
bz_1385038_fix_shared_drb_connections_after_fork
branch
from
ea7e425
to
271ae40
Compare
|
Okay updated the commit message per #16953 (comment) and synchronize on the |
|
@agrare can we have a method called |
|
Checked commit agrare@271ae40 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
This was referenced Feb 6, 2018
|
@agrare can you add the branches to backport this to? |
|
@jrafanie done, between the original BZ and the 3 clones this PR has a combined PM score of 30,820 😆 |
simaishi
pushed a commit
that referenced
this pull request
Feb 6, 2018
…ctions_after_fork Close open connections from parent after fork (cherry picked from commit b6062af) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1542735
|
Gaprindashvili backport details: |
simaishi
pushed a commit
that referenced
this pull request
Feb 6, 2018
…ctions_after_fork Close open connections from parent after fork (cherry picked from commit b6062af) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1481378
|
Fine backport details: |
|
I opened https://bugs.ruby-lang.org/issues/14471 to reopen the existing ruby bug |
simaishi
pushed a commit
that referenced
this pull request
Feb 26, 2018
…ctions_after_fork Close open connections from parent after fork (cherry picked from commit b6062af) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1481677
|
Euwe backport details: |
d-m-u
pushed a commit
to d-m-u/manageiq
that referenced
this pull request
Jun 6, 2018
…rb_connections_after_fork Close open connections from parent after fork (cherry picked from commit b6062af) Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1481378
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.
DRb::DRbConn keeps a global pool of open connections which is shared by
child processes when they are forked from a parent. If this parent
executes a DRb call prior to forking a child process the child picks up
this open connection and uses it which can cause replies from the server
to go to the wrong DRb client.
The connection pool in question is here
There is a long standing ruby bug https://bugs.ruby-lang.org/issues/2718
which describes the issue and has reproducer code attached.
This is the reproducer code that we used: https://gist.github.com/agrare/d9484884bd297b1615814128129cfc5c
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1385038