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

improve log description of QuorumController #15926

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

chickenchickenlove
Copy link
Contributor

Motivation

  • QuorumController warn with "Performing controller activation. Loaded ZK migration state of NONE" even if it's de-novo KRaft cluster. That message makes operator confused, and operator might waste time investigating that log. If we add some description to log, operator will be more convenient.

Modification

  • Add a log for de-novo KRaft cluster when zookeeper.metadata.migration.enable=false and the broker version supports KRaft.

Result

@chickenchickenlove
Copy link
Contributor Author

Hey, @mumrah !
I create PR to improve log desrcription when zookeeper.metadata.migration.enable is false.

When you have free time, could you take a look? 🙇‍♂️

Copy link
Contributor

@mumrah mumrah left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up @chickenchickenlove 👍

Left a suggestion for the log message

@@ -165,6 +165,8 @@ static ControllerResult<Void> recordsForNonEmptyLog(
throw new RuntimeException("Should not have ZK migrations enabled on a cluster that was " +
"created in KRaft mode.");
}
logMessageBuilder
.append("since this is a de-novo KRaft cluster.");
Copy link
Contributor

Choose a reason for hiding this comment

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

How about "This is expected because this is a de-novo KRaft cluster." ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is better 👍
I make a new commit to apply your comments.
Please take another look, when you have free time 🙇‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants