Skip to content

Updates FailedToCommitClusterStateException javadoc #136204

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
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Updates FailedToCommitClusterStateException javadoc #136204

wants to merge 2 commits into from
+1 −0

Conversation

joshua-adams-1
Copy link
Contributor

Update the FailedToCommitClusterStateException javadoc to specify that it can only be thrown by master nodes.

Relates to: ES-13061

@joshua-adams-1
Updates FailedToCommitClusterStateException javadoc
85e6d5b 
Update the FailedToCommitClusterStateException javadoc to specify that
it can only be thrown by master nodes.
@joshua-adams-1
Copy link
Contributor Author

I have been working on tightening up where the FailedToCommitClusterStateException is thrown inside the Coordinator.
Once #136083 is merged, it will be true that only master nodes can throw a FailedToCommitClusterStateException, hence the javadoc update

@joshua-adams-1 joshua-adams-1 self-assigned this Oct 8, 2025
@joshua-adams-1 joshua-adams-1 added :Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. >refactoring labels Oct 8, 2025
@joshua-adams-1 joshua-adams-1 marked this pull request as ready for review October 8, 2025 17:13
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Oct 8, 2025
@elasticsearchmachine
Copy link
Collaborator

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

DaveCTurner
DaveCTurner reviewed Oct 8, 2025
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

It's not quite correct as-is (see inline comment) and I'm not sure it's useful if corrected.

DaveCTurner
DaveCTurner reviewed Oct 8, 2025
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

It's not quite correct as-is (see inline comment) and I'm not sure it's useful if corrected.

...rc/main/java/org/elasticsearch/cluster/coordination/FailedToCommitClusterStateException.java

/**
* Exception indicating a cluster state update was published and may or may not have been committed.
* This exception can only be thrown by the master node.
Copy link
Contributor

Choose a reason for hiding this comment

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

"The" master node suggests you mean the currently-elected master, but if an elected master node cannot commit a cluster state update then it already isn't really the elected master node any more (and the cluster may already have elected a different node as the master).

But then if we just say "a" master node then it doesn't really mean much: this fact is implied by the fact that it's a consequence of publishing a state update, which is only done by the elected master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@joshua-adams-1 joshua-adams-1

Labels
:Distributed Coordination/Distributed A catch all label for anything in the Distributed Coordination area. Please avoid if you can. >refactoring Team:Distributed Coordination Meta label for Distributed Coordination team v9.3.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@joshua-adams-1 @elasticsearchmachine @DaveCTurner