Skip to content

Reject openConnection attempt while closing#86315

Merged
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2022-04-30-openConnection-while-closing
May 2, 2022
Merged

Reject openConnection attempt while closing#86315
DaveCTurner merged 2 commits intoelastic:masterfrom
DaveCTurner:2022-04-30-openConnection-while-closing

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

Today while the ClusterConnectionManager is closing it will reject
attempts to open managed connections (i.e. using connectToNode), but
it still permits ad-hoc connections (i.e. using openConnection). This
commit extends the existing refcounting mechanism to cover both cases,
preventing all concurrent connection attempts while shutting down.

Closes #86249
Relates #77539

Today while the `ClusterConnectionManager` is closing it will reject
attempts to open _managed_ connections (i.e. using `connectToNode`), but
it still permits ad-hoc connections (i.e. using `openConnection`). This
commit extends the existing refcounting mechanism to cover both cases,
preventing all concurrent connection attempts while shutting down.

Closes elastic#86249
Relates elastic#77539
@DaveCTurner DaveCTurner added >bug :Distributed/Network Http and internode communication implementations v8.3.0 labels Apr 30, 2022
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Apr 30, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @DaveCTurner, I've created a changelog YAML for you.

@DaveCTurner
Copy link
Copy Markdown
Member Author

In some sense this is the wrong place to apply this fix: it would also work to address this in TcpTransport itself (see #77539). But then again we only ever open connections via the connection manager and we need some protection at this level anyway, so after making this change we could simplify TcpTransport so that it doesn't even need to try and handle connection attempts during shutdown.

@DaveCTurner DaveCTurner merged commit 22136f0 into elastic:master May 2, 2022
@DaveCTurner DaveCTurner deleted the 2022-04-30-openConnection-while-closing branch May 2, 2022 07:49
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request May 17, 2022
We introduced an assertion in elastic#85131 to check the assumption that there
are no pending non-local response handlers when the `TransportService`
shuts down. We later discovered that this assertion rarely tripped,
which we fixed in elastic#86315, but that fix did not go into 8.2 so there are
still rare failures on this branch. This commit drops the faulty
assertion in 8.2.

Relates elastic#86293
elasticsearchmachine pushed a commit that referenced this pull request May 17, 2022
We introduced an assertion in #85131 to check the assumption that there
are no pending non-local response handlers when the `TransportService`
shuts down. We later discovered that this assertion rarely tripped,
which we fixed in #86315, but that fix did not go into 8.2 so there are
still rare failures on this branch. This commit drops the faulty
assertion in 8.2.

Relates #86293
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed/Network Http and internode communication implementations Team:Distributed Meta label for distributed team. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incomplete remote response handler after transport close in integration tests

4 participants