bump grpc to 1.60.1, reconnect idle connections #3147
Conversation
openshift-ci
bot
added
the
approved
| @@ -228,6 +228,11 @@ func (s *SourceStore) watch(ctx context.Context, key registry.CatalogKey, source | |||
| s.sources[key] = *src | |||
| s.sourcesLock.Unlock() | |||
|
|
|||
| // always try to reconnect if the connection goes idle | |||
| if newState == connectivity.Idle { | |||
| source.Conn.Connect() | |||
There was a problem hiding this comment.
Looks like this already no-ops if the underlying connections are not idle, so it might be better to call this unconditionally. A comment to note that this is non-blocking would be useful as well as it impacts the understanding of what consumers of s.notify see.
There was a problem hiding this comment.
Trying this in #3144, which should bring OLM to the minimal version of GRPC necessary for the next set of updates
joelanford
force-pushed
the
fix-flake-3146
branch
from
871d201 to
69ab783
Compare
Signed-off-by: Joe Lanford <[email protected]>
joelanford
force-pushed
the
fix-flake-3146
branch
from
69ab783 to
37746a3
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joelanford The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joelanford The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description of the change:
When we detect that the catalog operator's GPRC client connections become IDLE, we now always attempt to reconnect. This wouldn't typically be necessary for GRPC connections (GRPC clients automatically reconnect to the server when the next RPC call is made). However OLM reports the state of the GRPC connection in the CatalogSource API and makes assumptions about the connection being READY (or not).
IDLE connections are sort of in an unknown limbo. For example, one reason for an idle connection is if the server disconnects. In that case the client doesn't actually know if the server is still there until it tries to reconnect. Another reason is if the client has not made an RPC call within the idle timeout duration. There is no way to distinguish looking at the connection state the reason that the connection is idle. OLM clients want to know whether the server is actually present and connected, so this PR ensures that we attempt re-establishment of the TCP connection when the GRPC connection goes idle.
Motivation for the change:
Fixes #3146
Also unblocks us from upgrading other dependencies that are incompatible with grpc v1.40
Architectural changes:
None
Testing remarks:
Reviewer Checklist
/doc[FLAKE]are truly flaky and have an issue