flowinfra: cancel remote flows when node is drained #82752
Conversation
|
This change is |
|
cc @miretskiy - curious if you cherry-pick this PR to try with your test (without the change to the context in the change aggregator). |
yuzefovich
force-pushed
the
flow-drain
branch
2 times, most recently
from
dacf0e6 to
06d4c89
Compare
yuzefovich
force-pushed
the
flow-drain
branch
3 times, most recently
from
163b0ef to
7ae1e25
Compare
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @yuzefovich)
pkg/sql/distsql/server.go line 158 at r1 (raw file):
"determines whether the queries that are still running on the node that "+ "is being drained after waiting for 'server.shutdown.query_wait' are "+ "forcefully canceled",
I think this might read better:
determines whether queries that are still running on a node being drained are forcefully canceled after waiting the 'server.shutdown.query_wait' period.
yuzefovich
force-pushed
the
flow-drain
branch
2 times, most recently
from
e6ef8d7 to
71c78d1
Compare
There was a problem hiding this comment.
Added a release note.
TFTR!
bors r+
Reviewable status:
pkg/sql/distsql/server.go line 158 at r1 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
I think this might read better:
determines whether queries that are still running on a node being drained are forcefully canceled after waiting the 'server.shutdown.query_wait' period.
I agree, done.
This commit fixes an oversight in the draining process of the DistSQL flows. Previously, it was possible for some flows to keep on running even after `server.shutdown.query_wait` has passed (which acts as a grace period to allow queries to complete). This only affects the distributed queries since local queries are already canceled when the connections to the node being drained are interrupted. This commit makes it so that the flow registry actively cancels all still running flows after the query wait grace period. This is done by canceling the context of the flow. As a result, distributed queries that have flows on the node being drained now will result in an error (previously, they could stall the draining process until they would complete). Additionally, this commit fixes an oversight introduced in 5ff1974 so that all flows (except for fully-local queries) get registered with the flow registry. This matters for remote flows that don't have any inbound connections (e.g. `SELECT count(*)` query or a CDC flow) which would previously by-pass the flow registry altogether. In order to have an escape hatch in case the new behavior becomes problematic, a new private cluster setting is introduced that can disable the new behavior of canceling the still running flows. Release note (bug fix): When a CockroachDB node is being, all queries that are still running on that node are now forcefully canceled after waiting the `server.shutdown.query_wait` period.
|
Need to increase the max number of settings. |
|
bors r- |
|
bors r+ |
|
Canceled. |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
Build succeeded: |
This commit fixes an oversight in the draining process of the DistSQL
flows. Previously, it was possible for some flows to keep on running
even after
server.shutdown.query_waithas passed (which acts asa grace period to allow queries to complete). This only affects the
distributed queries since local queries are already canceled when the
connections to the node being drained are interrupted.
This commit makes it so that the flow registry actively cancels all
still running flows after the query wait grace period. This is done by
canceling the context of the flow. As a result, distributed queries that
have flows on the node being drained now will result in an error
(previously, they could stall the draining process until they would
complete).
Additionally, this commit fixes an oversight introduced in
5ff1974 so that all flows (except for
fully-local queries) get registered with the flow registry. This matters
for remote flows that don't have any inbound connections (e.g.
SELECT count(*)query or a CDC flow) which would previously by-passthe flow registry altogether.
In order to have an escape hatch in case the new behavior becomes
problematic, a new private cluster setting is introduced that can
disable the new behavior of canceling the still running flows.
Fixes: #82765.
Release note (bug fix): When a CockroachDB node is being, all queries
that are still running on that node are now forcefully canceled after
waiting the
server.shutdown.query_waitperiod.