-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
KAFKA-7838: Log leader and follower end offsets when shrinking ISR #6168
Conversation
Example output from log:
|
val newInSyncReplicas = inSyncReplicas -- outOfSyncReplicas | ||
assert(newInSyncReplicas.nonEmpty) | ||
info("Shrinking ISR from %s to %s".format(inSyncReplicas.map(_.brokerId).mkString(","), | ||
newInSyncReplicas.map(_.brokerId).mkString(","))) | ||
debug(s"Leader (hwm: ${leaderReplica.highWatermark.messageOffset}, endOffset: ${leaderReplica.logEndOffset.messageOffset})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I wouldn't use abbreviations such as hwn. I think it's good to call it "highWatermark". It seems nicer.
@junrao could you please take a look to see if this looks good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhruvilshah3 : Thanks for the patch. Just one comment below.
val newInSyncReplicas = inSyncReplicas -- outOfSyncReplicas | ||
assert(newInSyncReplicas.nonEmpty) | ||
info("Shrinking ISR from %s to %s".format(inSyncReplicas.map(_.brokerId).mkString(","), | ||
newInSyncReplicas.map(_.brokerId).mkString(","))) | ||
debug(s"Leader (highWatermark: ${leaderReplica.highWatermark.messageOffset}, endOffset: ${leaderReplica.logEndOffset.messageOffset})") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we just include the debug logging as part of the info logging above since it's useful info and doesn't add much additional logging overhead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dhruvilshah3 : Thanks for the updated PR. LGTM
* ak/trunk: MINOR: Update usage of deprecated API (apache#6146) KAFKA-4217: Add KStream.flatTransform (apache#5273) MINOR: Update Gradle to 5.1.1 (apache#6160) KAFKA-3522: Generalize Segments (apache#6170) Added quotes around the class path (apache#4469) KAFKA-7837: Ensure offline partitions are picked up as soon as possible when shrinking ISR (apache#6202) MINOR: In the MetadataResponse schema, ignorable should be a boolean KAFKA-7838: Log leader and follower end offsets when shrinking ISR (apache#6168) KAFKA-5692: Change PreferredReplicaLeaderElectionCommand to use Admin… (apache#3848) MINOR: clarify why suppress can sometimes drop tombstones (apache#6195) MINOR: Upgrade ducktape to 0.7.5 (apache#6197) MINOR: Improve IntegrationTestUtils documentation (apache#5664) MINOR: upgrade to jdk8 8u202 KAFKA-7693; Fix SequenceNumber overflow in producer (apache#5989) KAFKA-7692; Fix ProducerStateManager SequenceNumber overflow (apache#5990) MINOR: update copyright year in the NOTICE file. (apache#6196) KAFKA-7793: Improve the Trogdor command line. (apache#6133)
…pache#6168) Reviewers: Jun Rao <junrao@gmail.com>
No description provided.