Skip to content

Warn on not enough masters during election #20063

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 18, 2017
Merged

Warn on not enough masters during election #20063

merged 1 commit into from
Apr 18, 2017

Conversation

@pickypg
Copy link
Member

@pickypg pickypg commented Aug 18, 2016

This changes the trace level logging to warn, and adds the needed number to the message as well.

My fear is that it may get noisy, but this is an issue that you want to be noisy.

Closes #8362

@pickypg pickypg added review :Core/Infra/Logging Log management and logging utilities v5.0.0-beta1 labels Aug 18, 2016
@pickypg
Copy link
Member Author

pickypg commented Aug 18, 2016

/cc @jasontedor I can wait until log4j2 gets merged since this will be another thing for you to battle.

@pickypg pickypg force-pushed the feature/warn-on-minimum-masters branch from 9e76f42 to 54b21b5 Compare August 27, 2016 18:48
@dakrone
Copy link
Member

dakrone commented Sep 14, 2016

@pickypg this probably needs a rebase now that the log4j2 stuff is in, right?

@clintongormley
Copy link
Contributor

clintongormley commented Nov 26, 2016

@pickypg want to rebase and get this in?

@pickypg pickypg force-pushed the feature/warn-on-minimum-masters branch from 54b21b5 to 84ab26b Compare November 28, 2016 19:52
@pickypg
Copy link
Member Author

pickypg commented Nov 28, 2016

@clintongormley Done

bleskes
bleskes approved these changes Nov 29, 2016
View reviewed changes
Copy link
Contributor

@bleskes bleskes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left two nits.

core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java Outdated
Copy link
Contributor

@bleskes bleskes Nov 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be confusing - maybe "not enough master nodes (has [{}], needed [{}])"?

core/src/main/java/org/elasticsearch/discovery/zen/ZenDiscovery.java Outdated
Copy link
Contributor

@bleskes bleskes Nov 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we promote this to first class message, I think we should make it more user friendly. how about "not enough master nodes discovered during pinging (found [{}], needed [{}]), pinging again" ?

@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 23, 2017

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@dakrone
Copy link
Member

dakrone commented Apr 7, 2017

@pickypg are you going to pick (hah, get it?) this back up?

@pickypg
Copy link
Member Author

pickypg commented Apr 17, 2017

@dakrone Yeah, I'll rebase and make changes today.

@pickypg
Warn on not enough masters during election
00a4cc0 
This changes the trace level logging to warn, and adds the needed number to the message as well.

My fear is that it may get noisy, but this is an issue that you want to be noisy.
@pickypg pickypg force-pushed the feature/warn-on-minimum-masters branch from 84ab26b to 00a4cc0 Compare April 17, 2017 23:52
@pickypg
Copy link
Member Author

pickypg commented Apr 17, 2017 •
edited
Loading

@dakrone / @bleskes rebased on master and finally updated; I'll commit once it passes

@pickypg pickypg added the v5.5.0 label Apr 18, 2017
@pickypg pickypg merged commit 12c8423 into elastic:master Apr 18, 2017
@pickypg pickypg deleted the feature/warn-on-minimum-masters branch April 18, 2017 02:18
pickypg added a commit that referenced this pull request Apr 18, 2017
@pickypg
Warn on not enough masters during election (#20063)
23ff820 
This changes the trace level logging to warn, and adds the needed number to the message as well.

My fear is that it may get noisy, but this is an issue that you want to be noisy.
pickypg added a commit that referenced this pull request Apr 18, 2017
@pickypg
Warn on not enough masters during election (#20063)
edee015 
This changes the trace level logging to warn, and adds the needed number to the message as well.

My fear is that it may get noisy, but this is an issue that you want to be noisy.
@pickypg
Copy link
Member Author

pickypg commented Apr 18, 2017

5.x/5.5: 23ff820
5.4: edee015

bleskes added a commit that referenced this pull request Apr 19, 2017
@bleskes
ElectMasterService.hasEnoughMasterNodes should return false if no mas…
92af92a 
…ters were found

This is a regression introduced in #20063
bleskes added a commit that referenced this pull request Apr 19, 2017
@bleskes
ElectMasterService.hasEnoughMasterNodes should return false if no mas…
09193d8 
…ters were found

This is a regression introduced in #20063
bleskes added a commit that referenced this pull request Apr 19, 2017
@bleskes
ElectMasterService.hasEnoughMasterNodes should return false if no mas…
8758c54 
…ters were found

This is a regression introduced in #20063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@bleskes bleskes bleskes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Core/Infra/Logging Log management and logging utilities >enhancement v5.4.0 v5.5.0 v6.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pickypg @dakrone @clintongormley @elasticmachine @bleskes