server: declare node ready while decommissioning #43889
Merged
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
|
This change is |
|
started a discussion about the change here |
|
I got a nod from @asubiotto on the mailing list, so I think this is good to go |
Before this patch, a decommissioning or draining, but otherwise healthy, node would return an error from the /health?ready=1 endpoint, thus declaring itself unready and signaling load balancers to send traffic away. This patch makes it so that the decommissioning status no longer matters for the readyness determination. Draining nodes continue to declare themselves unready. Note that decommissioning nodes typically go through draining at the end of the process. The justification is that a node can be decommissioning for an arbitrary amount of time. During that time it can continue to hold leases, etc. So trying to avoid traffic is not particularly desirable. In fact, some people even want to keep a node in a decommissioning state indefinitely (by restarting a decommissioning node without recommissioning it). Also, the server.shutdown.drain_wait cluster setting is there to give load balancers ample time to find out about a draining node. Also, tactically, the code is simplified. Release note (general change): A node no longer declares itself to not be ready through the /health/ready=1 endpoint while it's in the process of decommissioning. It continues to declare itself unready while draining.
andreimatei
force-pushed
the
liveness.ready-on-decommissioning
branch
from
59f874a
to
139bd21
Compare
ajwerner
approved these changes
Jan 14, 2020
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @asubiotto)
ajwerner
approved these changes
Jan 14, 2020
There was a problem hiding this comment.
Reviewable status:
andreimatei
commented
Jan 14, 2020
There was a problem hiding this comment.
bors r+
Reviewable status:
Build failed |
craig bot
pushed a commit
that referenced
this pull request
Jan 14, 2020
43417: sql: remove restriction of foreign key on multiple outbound columns r=lucy-zhang a=otan Resolves #38850. Removing the restriction of foreign keys on multiple outbound columns. There is one minor issue we need to address which is done here -- if the constraint name already exists, append a number next to it as postgres does as well. We can make this fancier if we wish later. This PR comes with a test suite of corner cases that we suspect may be hinted at being wrong. It uses both legacy and the new optimizer settings for foreign key checks. Release note (sql change): We previously had a restriction that foreign keys can only reference one outbound column at any given point in time, e.g. in `CREATE TABLE test(a int)`, we cannot have two foreign keys on column a. This PR removes this restriction. 43889: server: declare node ready while decommissioning r=andreimatei a=andreimatei Before this patch, a decommissioning or draining, but otherwise healthy, node would return an error from the /health?ready=1 endpoint, thus declaring itself unready and signaling load balancers to send traffic away. This patch makes it so that the decommissioning status no longer matters for the readyness determination. Draining nodes continue to declare themselves unready. Note that decommissioning nodes typically go through draining at the end of the process. The justification is that a node can be decommissioning for an arbitrary amount of time. During that time it can continue to hold leases, etc. So trying to avoid traffic is not particularly desirable. In fact, some people even want to keep a node in a decommissioning state indefinitely (by restarting a decommissioning node without recommissioning it). Also, the server.shutdown.drain_wait cluster setting is there to give load balancers ample time to find out about a draining node. Also, tactically, the code is simplified. Release note (general change): A node no longer declares itself to not be ready through the /health/ready=1 endpoint while it's in the process of decommissioning. It continues to declare itself unready while draining. Co-authored-by: Oliver Tan <otan@cockroachlabs.com> Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
Build succeeded |
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.
Before this patch, a decommissioning or draining, but otherwise healthy,
node would return an error from the /health?ready=1 endpoint, thus
declaring itself unready and signaling load balancers to send traffic
away. This patch makes it so that the decommissioning status no longer
matters for the readyness determination. Draining nodes continue to
declare themselves unready.
Note that decommissioning nodes typically go through draining at the
end of the process.
The justification is that a node can be decommissioning for an arbitrary
amount of time. During that time it can continue to hold leases, etc. So
trying to avoid traffic is not particularly desirable. In fact, some
people even want to keep a node in a decommissioning state indefinitely
(by restarting a decommissioning node without recommissioning it).
Also, the server.shutdown.drain_wait cluster setting is there to give
load balancers ample time to find out about a draining node.
Also, tactically, the code is simplified.
Release note (general change): A node no longer declares itself to not
be ready through the /health/ready=1 endpoint while it's in the process
of decommissioning. It continues to declare itself unready while
draining.