VixDiskLib: Use a well-known IO to pass the URI #4318
Closed
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
As we can be more confident it's the right thing now, we can lose the 'URI:' prefix. In passing, also fix retries: previously, we would've attempted to re-read the URI upon retry, when it had already been consumed. That clearly wasn't going to work.
|
Checked commit matthewd@523f601 with ruby 1.9.3, rubocop 0.33.0, and haml-lint 0.13.0 |
jerryk55
added a commit
to jerryk55/manageiq
that referenced
this pull request
Sep 17, 2015
Added a thread to the VixDiskLibServer to read one end of a pipe that the SmartProxyWorker has opened. When the read returns the worker process has exited so the VixDiskLibServer shuts down. This fixes the BZ https://bugzilla.redhat.com/show_bug.cgi?id=1258985 This fix was implemented in lieu of previous PR ManageIQ#4277 - ManageIQ#4277 which passed SIGTERM from the worker process to the server process. The new implementation will handle any signal that has caused the worker to exit including SIGKILL. The previous PR will be closed. Instead of using an environment variable to let VixDiskLibServer know which FD contains the pipe we are using a known FD. In addition we have changed the previous logic for the other pipe being used by these two processes which is used to hand the DRb URI back to the worker so that it also uses a known FD. Finally we have fixed the retry logic in the worker to read the URI before attempting to establish the DRb connection. The "known FD" and retry logic code come from PR ManageIQ#4318 by @matthewd which will be closed in lieu of this new one.
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.
As we can be more confident it's the right thing now, we can lose the 'URI:' prefix.
In passing, also fix retries: previously, we would've attempted to re-read the URI upon retry, when it had already been consumed. That clearly wasn't going to work.
@jerryk55 this is entirely untested, mostly because I wouldn't know where to start. Feel free to use/ignore as you see fit...
If it were just some existing isolated code I may not have bothered, but as it sounds like we're about to replicate the pattern, it seemed worth a mention. (Also, I really don't see how that retry could possibly work.)