cmccabe commented on a change in pull request #10564:
URL: https://github.com/apache/kafka/pull/10564#discussion_r618133367
##########
File path:
metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java
##########
@@ -172,47 +174,54 @@ String diff(PartitionControlInfo prev) {
StringBuilder builder = new StringBuilder();
String prefix = "";
if (!Arrays.equals(replicas, prev.replicas)) {
-
builder.append(prefix).append("oldReplicas=").append(Arrays.toString(prev.replicas));
+ builder.append(prefix).append("replicas: ").
+ append(Arrays.toString(prev.replicas)).
+ append(" -> ").append(Arrays.toString(replicas));
prefix = ", ";
-
builder.append(prefix).append("newReplicas=").append(Arrays.toString(replicas));
}
if (!Arrays.equals(isr, prev.isr)) {
-
builder.append(prefix).append("oldIsr=").append(Arrays.toString(prev.isr));
+ builder.append(prefix).append("isr: ").
+ append(Arrays.toString(prev.isr)).
+ append(" -> ").append(Arrays.toString(isr));
prefix = ", ";
-
builder.append(prefix).append("newIsr=").append(Arrays.toString(isr));
}
if (!Arrays.equals(removingReplicas, prev.removingReplicas)) {
- builder.append(prefix).append("oldRemovingReplicas=").
- append(Arrays.toString(prev.removingReplicas));
+ builder.append(prefix).append("removingReplicas: ").
+ append(Arrays.toString(prev.removingReplicas)).
+ append(" -> ").append(Arrays.toString(removingReplicas));
prefix = ", ";
- builder.append(prefix).append("newRemovingReplicas=").
- append(Arrays.toString(removingReplicas));
}
if (!Arrays.equals(addingReplicas, prev.addingReplicas)) {
- builder.append(prefix).append("oldAddingReplicas=").
- append(Arrays.toString(prev.addingReplicas));
+ builder.append(prefix).append("addingReplicas: ").
+ append(Arrays.toString(prev.addingReplicas)).
+ append(" -> ").append(Arrays.toString(addingReplicas));
prefix = ", ";
- builder.append(prefix).append("newAddingReplicas=").
- append(Arrays.toString(addingReplicas));
}
if (leader != prev.leader) {
-
builder.append(prefix).append("oldLeader=").append(prev.leader);
+ builder.append(prefix).append("leader: ").
+ append(prev.leader).append(" -> ").append(leader);
prefix = ", ";
- builder.append(prefix).append("newLeader=").append(leader);
}
if (leaderEpoch != prev.leaderEpoch) {
-
builder.append(prefix).append("oldLeaderEpoch=").append(prev.leaderEpoch);
+ builder.append(prefix).append("leaderEpoch: ").
+ append(prev.leaderEpoch).append(" ->
").append(leaderEpoch);
prefix = ", ";
-
builder.append(prefix).append("newLeaderEpoch=").append(leaderEpoch);
}
if (partitionEpoch != prev.partitionEpoch) {
-
builder.append(prefix).append("oldPartitionEpoch=").append(prev.partitionEpoch);
- prefix = ", ";
-
builder.append(prefix).append("newPartitionEpoch=").append(partitionEpoch);
+ builder.append(prefix).append("partitionEpoch: ").
+ append(prev.partitionEpoch).append(" ->
").append(partitionEpoch);
}
return builder.toString();
}
+ void maybeLogPartitionChange(Logger log, String description,
PartitionControlInfo prev) {
+ if (!electionWasClean(leader, prev.isr)) {
+ log.info("UNCLEAN partition change for {}: {}", description,
diff(prev));
+ } else if (log.isDebugEnabled()) {
+ log.debug("partition change for {}: {}", description,
diff(prev));
Review comment:
I don't think this will be practical as the number of partitions grows.
A 10-node cluster with a million partitions could have hundreds of thousands of
partition changes when a node goes away or comes back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
