You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "lmr3796 (via GitHub)" <gi...@apache.org> on 2023/02/02 08:49:04 UTC

[GitHub] [kafka] lmr3796 opened a new pull request, #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

lmr3796 opened a new pull request, #13187:
URL: https://github.com/apache/kafka/pull/13187

   *More detailed description of your change,
   if necessary. The PR title and PR message become
   the squashed commit message, so use a separate
   comment to ping reviewers.*
   This is for better observability of URPs for troubleshooting.  
   When troubleshooting a URP issue, while the log already contains the out-of-sync replica's LogEndOffset, it would be helpful to also have the lastCaughtUpTime of the replica, which is also used for determining if it's in-sync.
   
   *Summary of testing strategy (including rationale)
   for the feature or bug fix. Unit and/or integration
   tests are expected for any behaviour change and
   system tests should be considered for larger changes.*
   No behavior changes other than logging.  CI should be sufficient
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] lmr3796 commented on pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "lmr3796 (via GitHub)" <gi...@apache.org>.
lmr3796 commented on PR #13187:
URL: https://github.com/apache/kafka/pull/13187#issuecomment-1424662401

   @clolov for visibility


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] divijvaidya merged pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya merged PR #13187:
URL: https://github.com/apache/kafka/pull/13187


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] lmr3796 commented on a diff in pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "lmr3796 (via GitHub)" <gi...@apache.org>.
lmr3796 commented on code in PR #13187:
URL: https://github.com/apache/kafka/pull/13187#discussion_r1235962927


##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -1046,10 +1046,14 @@ class Partition(val topicPartition: TopicPartition,
           partitionState match {
             case currentState: CommittedPartitionState if outOfSyncReplicaIds.nonEmpty =>
               val outOfSyncReplicaLog = outOfSyncReplicaIds.map { replicaId =>
-                val logEndOffsetMessage = getReplica(replicaId)
-                  .map(_.stateSnapshot.logEndOffset.toString)
+                val replicaStateSnapshot = getReplica(replicaId).map(_.stateSnapshot)
+                val logEndOffsetMessage = replicaStateSnapshot
+                  .map(_.logEndOffset.toString)
                   .getOrElse("unknown")
-                s"(brokerId: $replicaId, endOffset: $logEndOffsetMessage)"
+                val lastCaughtUpTimeMessage = replicaStateSnapshot
+                  .map(_.lastCaughtUpTimeMs.toString)
+                  .getOrElse("unknown")
+                s"(brokerId: $replicaId, endOffset: $logEndOffsetMessage, lastCaughtUpTime: $lastCaughtUpTimeMessage)"

Review Comment:
   Hi @divijvaidya thanks for the great idea.
   
   Just added the suffix



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] lmr3796 commented on pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "lmr3796 (via GitHub)" <gi...@apache.org>.
lmr3796 commented on PR #13187:
URL: https://github.com/apache/kafka/pull/13187#issuecomment-1413356998

   Hi @jsancio wondering if you can take a look at this change.
   
   We found it helpful for us to understand such URP issues.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] divijvaidya commented on a diff in pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "divijvaidya (via GitHub)" <gi...@apache.org>.
divijvaidya commented on code in PR #13187:
URL: https://github.com/apache/kafka/pull/13187#discussion_r1235943898


##########
core/src/main/scala/kafka/cluster/Partition.scala:
##########
@@ -1046,10 +1046,14 @@ class Partition(val topicPartition: TopicPartition,
           partitionState match {
             case currentState: CommittedPartitionState if outOfSyncReplicaIds.nonEmpty =>
               val outOfSyncReplicaLog = outOfSyncReplicaIds.map { replicaId =>
-                val logEndOffsetMessage = getReplica(replicaId)
-                  .map(_.stateSnapshot.logEndOffset.toString)
+                val replicaStateSnapshot = getReplica(replicaId).map(_.stateSnapshot)
+                val logEndOffsetMessage = replicaStateSnapshot
+                  .map(_.logEndOffset.toString)
                   .getOrElse("unknown")
-                s"(brokerId: $replicaId, endOffset: $logEndOffsetMessage)"
+                val lastCaughtUpTimeMessage = replicaStateSnapshot
+                  .map(_.lastCaughtUpTimeMs.toString)
+                  .getOrElse("unknown")
+                s"(brokerId: $replicaId, endOffset: $logEndOffsetMessage, lastCaughtUpTime: $lastCaughtUpTimeMessage)"

Review Comment:
   let's add unit to the time here. Otherwise it is difficult to figure out the unit sometimes. May I suggest changing changing name of lastCaughtUpTime to `lastCaughtUpTimeMs` instead? It is also consistent with https://github.com/apache/kafka/blob/474053d2973b8790e50ccfe1bb0699694b0de1c7/core/src/main/scala/kafka/cluster/Replica.scala#L178



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] github-actions[bot] commented on pull request #13187: MINOR: Log lastCaughtUpTime on ISR shrinkage

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13187:
URL: https://github.com/apache/kafka/pull/13187#issuecomment-1585813244

   This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has  merge conflicts, please update it with the latest from trunk (or appropriate release branch)
   If this PR is no longer valid or desired, please feel free to close it. If no activity occurrs in the next 30 days, it will be automatically closed.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org