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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate minimum_master_nodes #37868
Deprecate minimum_master_nodes #37868
Changes from 16 commits
cf9a419
7aed19d
9a952da
6dc78b1
186a16d
05af9a7
ba246f5
b8f9e45
8585d63
38332f4
4871b7e
dd30a2e
b9a64f5
79f7f41
7c58475
c2674ab
7da96d4
6f5bb43
7c8a540
9137ad2
f9fa2d2
140dfce
52fdcba
9f737b0
89bb05f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a comment why this is set to MAX_VALUE? Typically you would expect this to be unconfigured. Or we could randomize between unconfigured, configured to num_nodes / 2 + 1 and MAX_VALUE (in case user forgot to remove setting in upgrade)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced it with a more specific name in 9137ad2. The point is that there's no facility to remove a setting via this API, so we set it to an explicitly silly value and then assert that this is what we wanted when removing it here:
elasticsearch/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java
Lines 993 to 1001 in 9137ad2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about doing
.putNull(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey())
here and thenin
InternalTestCluster
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting it to
null
is a legitimate thing that a test might do, and I want to assert that this is not what has happened here.