internal/servers/controller: Worker failure connection cleanup #1340
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
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
709704e to
236fa0e
Compare
Base automatically changed from
vancluever/session-cleanup-worker
to
vancluever/session-cleanup
June 22, 2021 19:18
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
236fa0e to
33929c2
Compare
jefferai
reviewed
Jun 22, 2021
jefferai
reviewed
Jun 22, 2021
jefferai
reviewed
Jun 22, 2021
jefferai
reviewed
Jun 22, 2021
|
@jefferai feedback all makes sense, will work, on applying it tomorrow! |
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
33929c2 to
5549b1a
Compare
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
5549b1a to
7f0a27a
Compare
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
7f0a27a to
c6b3047
Compare
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
c6b3047 to
db2a864
Compare
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
db2a864 to
5ed52f6
Compare
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
5ed52f6 to
c6f1158
Compare
|
@jefferai this is ready for review again. I'll check in a bit to see if tests passed. 🤞 |
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
c6f1158 to
e73e6d4
Compare
louisruch
reviewed
Jul 13, 2021
This commit adds the support to do the following: * Mark connections for non-reporting workers as closed. This is the controller counterpart to the worker functionality (see #1330). This is written as a scheduled job that does the work DB-side in a single atomic query. * Works to reconcile states if such a broken controller-worker connection resumes and a worker reports a connection as connected that should be disconnected. In this case, the controller will send an update request, and the worker will honor it and terminate the connection. * Further refinement of the grace period setting has been added here. We have converged on the current server "liveness" setting as our default here, which is half of the previous 30s (15 seconds, in other words). Additionally, this is now configurable on the controller and worker side, with the caveat that it's currently impossible to do so in config as the setting has been untagged in HCL. This is exposed so that we can run some sophisticated testing scenarios where we skew the grace period to either the controller or worker to ensure the aforementioned reconciliation works. * Some repository functions have been added to support the new functionality, in addition to some test code to the worker to allow querying of session state while testing.
…uilding" This reverts commit 0dc2d4606dcb12478906dc4d9b6a7cf6c2e0de14.
vancluever
force-pushed
the
vancluever/session-cleanup-controller
branch
from
ebc0b96 to
b6ac810
Compare
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
jefferai
reviewed
Jul 14, 2021
vancluever
added a commit
that referenced
this pull request
Jul 23, 2021
* internal/servers/controller: refactor WaitForNextWorkerStatusUpdate * internal/servers/worker: use connId instead of conn.GetConnectionId() * internal/servers/worker: remove Worker.logClose * internal/session: complement versus inclusive state search * internal/session: use ScanRows instead of Scan * internal/session: make closeConnectionsForDeadServersCte results gormable * internal/tests/cluster: rename TestWorkerSessionCleanup to TestSessionCleanup * Simplify SQL query for CloseConnectionsForDeadWorkers (#1410) Co-authored-by: Michael Gaffney <mgaffney@users.noreply.github.com>
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.
This commit adds the support to do the following:
Mark connections for non-reporting workers as closed. This is the
controller counterpart to the worker functionality (see internal/servers/worker: Controller failure connection cleanup #1330). This is
written as a scheduled job that does most of the work DB-side, save some
rudimentary checking of individual workers' last update times.
Works to reconcile states if such a broken controller-worker
connection resumes and a worker reports a connection as connected that
should be disconnected. In this case, the controller will send an update
request, and the worker will honor it and terminate the connection.
Further refinement of the grace period setting has been added here.
We have converged on the current server "liveness" setting as our
default here, which is half of the previous 30s (15 seconds, in other
words). Additionally, this is now configurable on the controller and
worker side, with the caveat that it's currently impossible to do so in
config as the setting has been untagged in HCL. This is exposed so that
we can run some sophisticated testing scenarios where we skew the grace
period to either the controller or worker to ensure the aforementioned
reconciliation works.
Some repository functions have been added to support the new
functionality.