mumrah commented on code in PR #15918:
URL: https://github.com/apache/kafka/pull/15918#discussion_r1640047647
##########
metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java:
##########
@@ -230,16 +234,24 @@ private boolean areZkBrokersReadyForMigration() {
* @param name A descriptive name of the function that is being
applied
* @param migrationOp A function which performs some migration operations
and possibly transforms our internal state
*/
+
private void applyMigrationOperation(String name, KRaftMigrationOperation
migrationOp) {
+ applyMigrationOperation(name, migrationOp, false);
+ }
+
+ private void applyMigrationOperation(String name, KRaftMigrationOperation
migrationOp, boolean alwaysLog) {
ZkMigrationLeadershipState beforeState = this.migrationLeadershipState;
+ long startTimeNs = time.nanoseconds();
ZkMigrationLeadershipState afterState = migrationOp.apply(beforeState);
- if (afterState.loggableChangeSinceState(beforeState)) {
- log.info("{}. Transitioned migration state from {} to {}", name,
beforeState, afterState);
+ long durationNs = time.nanoseconds() - startTimeNs;
+ if (afterState.loggableChangeSinceState(beforeState) || alwaysLog) {
+ log.info("{} in {} ns. Transitioned migration state from {} to {}",
+ name, durationNs, beforeState, afterState);
Review Comment:
Yea, I think so. For the two use cases of `alwaysLog == true`, the states
will always be different. Principle of Least Surprise makes me thing a
developer seeing "alwaysLog" boolean will in fact always cause a log to be
produced
--
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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
