Reuse the daemon RPC connection for the whole request - #651
Open
jfpardy wants to merge 2 commits into
Open
Conversation
|
Thank you for your pull request. Before we can look at it, you'll need to sign a Contributor License Agreement (CLA). Please follow instructions at https://icinga.com/company/contributor-agreement to sign the CLA. After that, please reply here with a comment and we'll verify. Contributors that have not signed yet: @jfpardy Details
|
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
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.
Since #644 the sync RPC helper closes the daemon
client after every call. So the second
syncRpcCall()of a request, and everyone after it, dies with
Cannot write to JSON-RPC socket.Problem
close()nulls$connectionbut leaves$pendingConnectionholding a promisefor it, because
connect()repopulates it right after$connectedclears it.The next
connection()hands back that promise, resolves a connection that'salready closed, and rejects at
JsonRpcConnection::sendRequest()'s writabilityguard.
The daemon status page shows this behaviour.
VMware API Connectionsrenders fine,then
Pending HTTP Requestsstops working and log level control stops displaying(#647).Clearing
$pendingConnectionfixes the rejection, but then every call stillopens its own connection. I tried that first, and on my test deployment the daemon
would only answer a single status page load per restart. Going back to one
connection per request skips the reconnect entirely. The stream watcher from
#644 only has to be gone before the loop could next run, and
await()hasalready stopped the loop by then, so React's shutdown handler leaves it alone.
Changes
RemoteClientacross all RPC calls of a request and close it in ashutdown function, instead of a
finallyafter every call.$pendingConnectioninRemoteClient::close()too, outside theif,since a failed connect leaves
$connectionnull while the rejected promisesticks around.
closes #647