Support shutting down coordinator using /info/state endpoint#15992
Support shutting down coordinator using /info/state endpoint#15992tdcmeehan merged 1 commit intoprestodb:masterfrom
Conversation
|
|
|
I think you submitted the EasyCLA request incorrectly? You need to select Uber as your company. |
Sorry for the confusion, I had the primary email as my gmail account with Uber signature on linuxfoundation :) I made the changes to use FB email account as my primary email and went through the acknowledgement. |
abhiseksaikia
force-pushed
the
asaikia_coordinator_shutdown
branch
2 times, most recently
from
f42b1c1 to
6b3d8e3
Compare
abhiseksaikia
force-pushed
the
asaikia_coordinator_shutdown
branch
from
6b3d8e3 to
350d819
Compare
abhiseksaikia
changed the title
abhiseksaikia
force-pushed
the
asaikia_coordinator_shutdown
branch
from
350d819 to
459ea13
Compare
There was a problem hiding this comment.
use requireNonNull to fail fast.
There was a problem hiding this comment.
Makes sense, I have made the changes
presto-tests/src/test/java/com/facebook/presto/tests/TestGracefulShutdown.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
I think this can be done in just the worker path.
There was a problem hiding this comment.
I included this for coordinator to cover the cases where "node-scheduler.include-coordinator" is used as true. Please correct me if I am wrong here.
There was a problem hiding this comment.
I think it make sense to skip this for coordinator given query monitoring will consider tasks as well. So it becomes redundant for coordinator. So let's skip this for coordinator.
There was a problem hiding this comment.
This is not required, I have removed this.
There was a problem hiding this comment.
Please add a max time limit to this test
There was a problem hiding this comment.
Makes sense, added timeout for the test.
abhiseksaikia
force-pushed
the
asaikia_coordinator_shutdown
branch
2 times, most recently
from
6540b86 to
499993c
Compare
|
Test failure looks relevant. Otherwise LGTM. |
Currently we cannot shut down coordinator through /info/state endpoint.With the disagg coordinator setup, we should able to do this so that we can drain individual coordinators gracefully.
abhiseksaikia
force-pushed
the
asaikia_coordinator_shutdown
branch
from
499993c to
6989f0f
Compare
4 participants
Support shutting down coordinator using PUT /info/state
Currently we cannot shut down coordinator through /info/state endpoint. With the disagg coordinator setup, we should able to do this so that we can drain individual coordinators gracefully.
Test plan -
Use PUT /v1/info endpoint with payload "SHUTTING_DOWN"
Before the coordinator is shutdown, GET /v1/info/state returns "SHUTTING_DOWN" as the response.
Once the coordinator is shutdown (after grace period), server refuses connection to port used by coordinator
== RELEASE NOTES ==
General Changes