Skip to content
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

AddVotingConfigExclusionsAction not honoring max_voting_config_exclusions value from yml #53455

Closed
shwetathareja opened this issue Mar 12, 2020 · 3 comments · Fixed by #53717
Closed
Assignees
Labels
>bug :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection.

Comments

@shwetathareja
Copy link

Elasticsearch version (bin/elasticsearch --version): 7.1, 7.6

Description of the problem including expected versus actual behavior:

max_voting_config_exclusions setting is NodeScope but yml value is not honored in the TransportAddVotingConfigExclusionsAction and uses default value as 10 (in case persistent/ transient setting is not set)

public static final Setting<Integer> MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING
        = Setting.intSetting("cluster.max_voting_config_exclusions", 10, 1, Property.Dynamic, Property.NodeScope);

The reason for that TransportAddVotingConfigExclusionsAction is getting the settings object from cluster state metadata which is constructed using only persistant & transient setting and doesn't have the settings object which is created in Node.java using the yml file.

return request.resolveVotingConfigExclusionsAndCheckMaximum(state,
            MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.get(state.metaData().settings()), MAXIMUM_VOTING_CONFIG_EXCLUSIONS_SETTING.getKey());

from MetaData class constructor
this.settings = Settings.builder().put(persistentSettings).put(transientSettings).build();

Fix:
TransportAddVotingConfigExclusionsAction should have access to settings object constructed from yml and then applying persistent/ transient settings.

Steps to reproduce:

1. Update the yml with cluster.max_voting_config_exclusions set to 50
2. Verified, 50 is picked from yml - curl "localhost:9200/_cluster/settings?pretty&include_defaults"
{
 "persistent" : { },
 "transient" : { },
 "defaults" : {
   "cluster" : {
     "max_voting_config_exclusions" : "50",
     "auto_shrink_voting_configuration" : "true",
.....
3.  Try adding voting exclusion as - curl -X POST "localhost:9200/_cluster/voting_config_exclusions/attrib1:val1?pretty"
{
 "error" : {
   "root_cause" : [
     {
       "type" : "illegal_argument_exception",
       "reason" : "add voting config exclusions request for [attrib1:val1] would add [12] exclusions to the existing [0] which would exceed the maximum of [10] set by [cluster.max_voting_config_exclusions]"
     }
   ],
   "type" : "illegal_argument_exception",
   "reason" : "add voting config exclusions request for [attrib1:val1] would add [12] exclusions to the existing [0] which would exceed the maximum of [10] set by [cluster.max_voting_config_exclusions]"
 },
 "status" : 400
}



@danielmitterdorfer danielmitterdorfer added the :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. label Mar 18, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Cluster Coordination)

@DaveCTurner DaveCTurner self-assigned this Mar 18, 2020
@DaveCTurner
Copy link
Contributor

Thanks for the analysis @shwetathareja, I agree.

However, you should not need more than 10 tombstones as a matter of course. Clusters should have a lot less than 10 master-eligible nodes, and you should clear the tombstones as soon as they're no longer needed.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Mar 18, 2020
Today we only read `cluster.max_voting_config_exclusions` from the dynamic
settings in the cluster metadata, ignoring any value set in
`elasticsearch.yml`. This commit addresses this.

Closes elastic#53455
@shwetathareja
Copy link
Author

@DaveCTurner: Yes agreed, we should clear the tombstone entries.

Also, I have observed similar issue for cluster.blocks.read_only setting which is NodeScope but doesn't honor static value from yml.

DaveCTurner added a commit that referenced this issue Mar 23, 2020
Today we only read `cluster.max_voting_config_exclusions` from the dynamic
settings in the cluster metadata, ignoring any value set in
`elasticsearch.yml`. This commit addresses this.

Closes #53455
DaveCTurner added a commit that referenced this issue Mar 23, 2020
Today we only read `cluster.max_voting_config_exclusions` from the dynamic
settings in the cluster metadata, ignoring any value set in
`elasticsearch.yml`. This commit addresses this.

Closes #53455
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants