subject:"\[GitHub\] \[kafka\] junrao commented on a change in pull request #8479\: KAFKA\-9769\: Finish operations for leaderEpoch\-updated partitions up to point ZK Exception"

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-07-01 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r448527321



##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,15 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)

Review comment:
   @andrewchoi5 : I was asking if we could change 
   
   `this.createLogIfNotExists()`
   
   to
   
   `createLogIfNotExists()`
   
   





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-06-30 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r447927722



##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,15 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)

Review comment:
   Do we need this? Ditto below.





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-06-26 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r446316668



##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,16 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)
+  } catch {
+case e: ZooKeeperClientException =>
+  stateChangeLogger.info(s"A ZooKeeper client exception has occurred 
and makeLeader will be skipping the " +
+s"state change for the partition with leader epoch: $leaderEpoch 
", e)
+  error(s"ZooKeeper client error occurred while this partition was 
becoming the leader for $topicPartition.", e)

Review comment:
   I meant removing the line in 509 of error(s"ZooKeeper client .





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-06-26 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r446306280



##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,16 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)
+  } catch {
+case e: ZooKeeperClientException =>
+  stateChangeLogger.info(s"A ZooKeeper client exception has occurred 
and makeLeader will be skipping the " +
+s"state change for the partition with leader epoch: $leaderEpoch 
", e)
+  error(s"ZooKeeper client error occurred while this partition was 
becoming the leader for $topicPartition.", e)

Review comment:
   We probably can just keep the state change logging. Also, the logging 
level probably should be error instead of info. Ditto below.





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-06-25 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r445904547



##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,16 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)
+  } catch {
+case e: ZooKeeperClientException =>
+  stateChangeLogger.info(s"Because a ZooKeeper client exception has 
occurred, completed become leader " +
+s"state change from epoch $leaderEpoch only for those updated 
partitions with before " +

Review comment:
   The message seem inaccurate since we only skip this partition now. Ditto 
in makeFollower().

##
File path: core/src/main/scala/kafka/cluster/Partition.scala
##
@@ -499,7 +500,16 @@ class Partition(val topicPartition: TopicPartition,
 addingReplicas = addingReplicas,
 removingReplicas = removingReplicas
   )
-  createLogIfNotExists(partitionState.isNew, isFutureReplica = false, 
highWatermarkCheckpoints)
+  try {
+this.createLogIfNotExists(partitionState.isNew, isFutureReplica = 
false, highWatermarkCheckpoints)
+  } catch {
+case e: ZooKeeperClientException =>
+  stateChangeLogger.info(s"Because a ZooKeeper client exception has 
occurred, completed become leader " +
+s"state change from epoch $leaderEpoch only for those updated 
partitions with before " +
+s"ZooKeeper disconnect occurred.", e)
+  error(s"ZooKeeper client occurred while rendering a 
$topicPartition's leader through zkClient.'", e)

Review comment:
   Do we need ' before "?
   
   Also, the text could probably be sth like "ZooKeeper client error occurred 
while becoming leader for $topicPartition." 
   
   Ditto in makeFollower().





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

2020-06-24 Thread GitBox



junrao commented on a change in pull request #8479:
URL: https://github.com/apache/kafka/pull/8479#discussion_r445234702



##
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##
@@ -1556,6 +1557,11 @@ class ReplicaManager(val config: KafkaConfig,
 error(s"Error while making broker the follower for partition 
$partition with leader " +
   s"$newLeaderBrokerId in dir $dirOpt", e)
 responseMap.put(partition.topicPartition, 
Errors.KAFKA_STORAGE_ERROR)
+  case e: ZooKeeperClientException =>

Review comment:
   It's probably better to do this in Partition.makeFollower() instead of 
here. That way, we only skip partitions that have incurred ZK error.
   
   Also, the same ZK exception can happen in Partition.makeLeader(). So, we 
want to do the same thing there as well.





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

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

[GitHub] [kafka] junrao commented on a change in pull request #8479: KAFKA-9769: Finish operations for leaderEpoch-updated partitions up to point ZK Exception

6 matches

Site Navigation

Mail list logo

Footer information