Skip to content

Commit

Permalink
JsonRpcConnection: Don't drop client from cache prematurely
Browse files Browse the repository at this point in the history
PR #7445 incorrectly assumed that a peer that had already disconnected
and never reconnected was due to the endpoint client being dropped after
a successful socket shutdown. However, the issue at that time was that
there was not a single timeout guards that could cancel the `async_shutdown`
call, petentially blocking indefinetely. Although removing the client from
cache early might have allowed the endpoint to reconnect, it did not
resolve the underlying problem. Now that we have a proper cancellation
timeout, we can wait until the currently used socket is fully closed
before dropping the client from our cache. When our socket termination
works reliably, the `ApiListener` reconnect timer should attempt to
reconnect this endpoint after the next tick. Additionally, we now have
logs both for before and after socket termination, which may help
identify if it is hanging somewhere in between.
  • Loading branch information
yhabteab committed Oct 31, 2024
1 parent 92399a9 commit b7dfdd3
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions lib/remote/jsonrpcconnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,8 @@ void JsonRpcConnection::Disconnect()
JsonRpcConnection::Ptr keepAlive (this);

IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) {
Log(LogWarning, "JsonRpcConnection")
<< "API client disconnected for identity '" << m_Identity << "'";

// We need to unregister the endpoint client as soon as possible not to confuse Icinga 2,
// given that Endpoint::GetConnected() is just performing a check that the endpoint's client
// cache is not empty, which could result in an already disconnected endpoint never trying to
// reconnect again. See #7444.
if (m_Endpoint) {
m_Endpoint->RemoveClient(this);
} else {
ApiListener::GetInstance()->RemoveAnonymousClient(this);
}
Log(LogNotice, "JsonRpcConnection")
<< "Disconnecting API client for identity '" << m_Identity << "'";

m_OutgoingMessagesQueued.Set();

Expand Down Expand Up @@ -255,6 +245,15 @@ void JsonRpcConnection::Disconnect()
shutdownTimeout->Cancel();

m_Stream->lowest_layer().shutdown(m_Stream->lowest_layer().shutdown_both, ec);

if (m_Endpoint) {
m_Endpoint->RemoveClient(this);
} else {
ApiListener::GetInstance()->RemoveAnonymousClient(this);
}

Log(LogWarning, "JsonRpcConnection")
<< "API client disconnected for identity '" << m_Identity << "'";
});
}
}
Expand Down

0 comments on commit b7dfdd3

Please sign in to comment.