You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2024/10/18 15:21:28 UTC

[PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   Refer to [KAFKA-17339](https://issues.apache.org/jira/browse/KAFKA-17339) for more information.
   
   Enable the `LocalLogManager` class to really support prepared batches, improving the test implementation to more closely mimic the behavior of a real log manager(KafkaRaftClient). 
   
   ### Committer Checklist (excluded from commit message)
   - [x] Verify design and implementation 
   - [x] Verify test coverage and CI build status
   - [x] 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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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


##########
metadata/src/test/java/org/apache/kafka/metalog/LocalLogManager.java:
##########
@@ -725,19 +735,35 @@ public long prepareAppend(
         int epoch,
         List<ApiMessageAndVersion> batch
     ) {
+        if (!leader.isLeader(nodeId)) {
+            log.debug("prepareAppend(nodeId={}, epoch={}): the given node id does not " +
+                    "match the current leader id of {}.", nodeId, epoch, leader.leaderId());
+            throw new NotLeaderException("Append failed because the replication is not the current leader");
+        }
+
         if (batch.isEmpty()) {
             throw new IllegalArgumentException("Batch cannot be empty");
         }
 
         if (throwOnNextAppend.getAndSet(false)) {
             throw new BufferAllocationException("Test asked to fail the next prepareAppend");
         }
-
-        return shared.tryAppend(nodeId, epoch, batch);
+        long appendTimestamp = (shared.prevOffset + 1) * 10;

Review Comment:
   Sorry I didnt notice there is an existing one, just addressed, PTAL. Thanks!



-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   This PR has been closed since it has not had any activity in 120 days. If you feel like this
   was a mistake, or you would like to continue working on it, please feel free to re-open the 
   PR and ask for a review.


-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #17544: KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend
URL: https://github.com/apache/kafka/pull/17544


-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   After merging trunk, the test `QuorumControllerMetricsIntegrationTest#testFailingOverIncrementsNewActiveControllerCount (forceFailoverUsingLogLayer=true)` failed with following log. The first exception is expected, as `logEnv.activeLogManager().get().throwOnNextAppend()` has been called. But the subsequent SnapshotRegistry exception seems always happen 4 time, which leads the `NEW_ACTIVE_CONTROLLERS_COUNT` be 6 instead of 2.
   
   ```
   org.apache.kafka.raft.errors.BufferAllocationException: Test asked to fail the next prepareAppend
   	at org.apache.kafka.metalog.LocalLogManager.prepareAppend(LocalLogManager.java:743) ~[test/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.lambda$run$0(QuorumController.java:827) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController.appendRecords(QuorumController.java:912) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.run(QuorumController.java:821) ~[main/:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventContext.run(KafkaEventQueue.java:132) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.handleEvents(KafkaEventQueue.java:215) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.run(KafkaEventQueue.java:186) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
   16:27:39.012 [quorum-controller-1-event-handler] ERROR org.apache.kafka.server.fault.MockFaultHandler - Encountered nonFatalFaultHandler fault: createTopics: event failed with RuntimeException (treated as UnknownServerException) at epoch 3 in 47070 microseconds. Renouncing leadership and reverting to the last committed offset 5.
   java.lang.RuntimeException: Can't create a new in-memory snapshot at epoch 3 because there is already a snapshot with epoch 5. Snapshot epochs are 5
   	at org.apache.kafka.timeline.SnapshotRegistry.getOrCreateSnapshot(SnapshotRegistry.java:225) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.timeline.SnapshotRegistry.idempotentCreateSnapshot(SnapshotRegistry.java:245) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.controller.OffsetControlManager.handleScheduleAppend(OffsetControlManager.java:305) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.lambda$run$0(QuorumController.java:844) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController.appendRecords(QuorumController.java:912) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.run(QuorumController.java:821) ~[main/:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventContext.run(KafkaEventQueue.java:132) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.handleEvents(KafkaEventQueue.java:215) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.run(KafkaEventQueue.java:186) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
   16:27:48.077 [quorum-controller-2-event-handler] ERROR org.apache.kafka.server.fault.MockFaultHandler - Encountered nonFatalFaultHandler fault: maybeFenceStaleBroker: event failed with RuntimeException (treated as UnknownServerException) at epoch 5 in 26900 microseconds. Renouncing leadership and reverting to the last committed offset 9.
   java.lang.RuntimeException: Can't create a new in-memory snapshot at epoch 3 because there is already a snapshot with epoch 9. Snapshot epochs are 9
   	at org.apache.kafka.timeline.SnapshotRegistry.getOrCreateSnapshot(SnapshotRegistry.java:225) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.timeline.SnapshotRegistry.idempotentCreateSnapshot(SnapshotRegistry.java:245) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.controller.OffsetControlManager.handleScheduleAppend(OffsetControlManager.java:305) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.lambda$run$0(QuorumController.java:844) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController.appendRecords(QuorumController.java:928) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.run(QuorumController.java:821) ~[main/:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventContext.run(KafkaEventQueue.java:132) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.handleEvents(KafkaEventQueue.java:215) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.run(KafkaEventQueue.java:186) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
   16:27:57.133 [quorum-controller-0-event-handler] ERROR org.apache.kafka.server.fault.MockFaultHandler - Encountered nonFatalFaultHandler fault: maybeFenceStaleBroker: event failed with RuntimeException (treated as UnknownServerException) at epoch 7 in 29393 microseconds. Renouncing leadership and reverting to the last committed offset 13.
   java.lang.RuntimeException: Can't create a new in-memory snapshot at epoch 6 because there is already a snapshot with epoch 13. Snapshot epochs are 13
   	at org.apache.kafka.timeline.SnapshotRegistry.getOrCreateSnapshot(SnapshotRegistry.java:225) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.timeline.SnapshotRegistry.idempotentCreateSnapshot(SnapshotRegistry.java:245) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.controller.OffsetControlManager.handleScheduleAppend(OffsetControlManager.java:305) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.lambda$run$0(QuorumController.java:844) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController.appendRecords(QuorumController.java:928) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.run(QuorumController.java:821) ~[main/:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventContext.run(KafkaEventQueue.java:132) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.handleEvents(KafkaEventQueue.java:215) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.run(KafkaEventQueue.java:186) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
   16:28:06.204 [quorum-controller-2-event-handler] ERROR org.apache.kafka.server.fault.MockFaultHandler - Encountered nonFatalFaultHandler fault: maybeFenceStaleBroker: event failed with RuntimeException (treated as UnknownServerException) at epoch 9 in 32702 microseconds. Renouncing leadership and reverting to the last committed offset 16.
   java.lang.RuntimeException: Can't create a new in-memory snapshot at epoch 4 because there is already a snapshot with epoch 16. Snapshot epochs are 16
   	at org.apache.kafka.timeline.SnapshotRegistry.getOrCreateSnapshot(SnapshotRegistry.java:225) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.timeline.SnapshotRegistry.idempotentCreateSnapshot(SnapshotRegistry.java:245) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.controller.OffsetControlManager.handleScheduleAppend(OffsetControlManager.java:305) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.lambda$run$0(QuorumController.java:844) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController.appendRecords(QuorumController.java:928) ~[main/:?]
   	at org.apache.kafka.controller.QuorumController$ControllerWriteEvent.run(QuorumController.java:821) ~[main/:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventContext.run(KafkaEventQueue.java:132) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.handleEvents(KafkaEventQueue.java:215) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at org.apache.kafka.queue.KafkaEventQueue$EventHandler.run(KafkaEventQueue.java:186) ~[kafka-server-common-4.1.0-SNAPSHOT.jar:?]
   	at java.base/java.lang.Thread.run(Thread.java:1575) [?:?]
   
   Expected :2
   Actual   :6
   ```
   
   @ahuang98,
   Do you have any thoughts on this issue? Any feedback would be greatly appreciated. Thank you!
   


-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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


##########
metadata/src/test/java/org/apache/kafka/metalog/LocalLogManager.java:
##########
@@ -725,19 +735,35 @@ public long prepareAppend(
         int epoch,
         List<ApiMessageAndVersion> batch
     ) {
+        if (!leader.isLeader(nodeId)) {
+            log.debug("prepareAppend(nodeId={}, epoch={}): the given node id does not " +
+                    "match the current leader id of {}.", nodeId, epoch, leader.leaderId());
+            throw new NotLeaderException("Append failed because the replication is not the current leader");
+        }
+
         if (batch.isEmpty()) {
             throw new IllegalArgumentException("Batch cannot be empty");
         }
 
         if (throwOnNextAppend.getAndSet(false)) {
             throw new BufferAllocationException("Test asked to fail the next prepareAppend");
         }
-
-        return shared.tryAppend(nodeId, epoch, batch);
+        long appendTimestamp = (shared.prevOffset + 1) * 10;

Review Comment:
   why not use logEndOffset()? 



-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   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 leave a comment asking for a review. If the PR has 
   merge conflicts, update it with the latest from the base branch.
   <p>
   If you are having difficulty finding a reviewer, please reach out on the 
   [mailing list](https://kafka.apache.org/contact).
   <p>
   If this PR is no longer valid or desired, please feel free to close it. If no activity
   occurs 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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   @peterxcli Could you help to resolve this conflict and re-trigger CI?


-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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


##########
metadata/src/test/java/org/apache/kafka/metalog/LocalLogManager.java:
##########
@@ -732,12 +742,22 @@ public long prepareAppend(
         if (throwOnNextAppend.getAndSet(false)) {
             throw new BufferAllocationException("Test asked to fail the next prepareAppend");
         }
-

Review Comment:
   Should we also need to check current node is leader here?
   https://github.com/apache/kafka/blob/0f7cd4dcdeb2c705c01743927e36b66b06010f20/raft/src/main/java/org/apache/kafka/raft/RaftClient.java#L150



-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   Hi @jsancio,
   
   I hope you're doing well. I wanted to kindly ask if you could take a moment to review this pull request
   Whenever you have time, your feedback would be greatly appreciated!
   Thank you so much for your help.
   
   


-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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

   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 leave a comment asking for a review. If the PR has 
   merge conflicts, update it with the latest from the base branch.
   <p>
   If you are having difficulty finding a reviewer, please reach out on the 
   [mailing list](https://kafka.apache.org/contact).
   <p>
   If this PR is no longer valid or desired, please feel free to close it. If no activity
   occurs 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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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


##########
metadata/src/test/java/org/apache/kafka/metalog/LocalLogManager.java:
##########
@@ -732,12 +742,22 @@ public long prepareAppend(
         if (throwOnNextAppend.getAndSet(false)) {
             throw new BufferAllocationException("Test asked to fail the next prepareAppend");
         }
-

Review Comment:
   Thanks!



-- 
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


Re: [PR] KAFKA-17339: LocalLogManager should support RaftClient#schedulePreparedAppend [kafka]

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


##########
metadata/src/test/java/org/apache/kafka/metalog/LocalLogManager.java:
##########
@@ -732,12 +742,22 @@ public long prepareAppend(
         if (throwOnNextAppend.getAndSet(false)) {
             throw new BufferAllocationException("Test asked to fail the next prepareAppend");
         }
-

Review Comment:
   Should we also need to check current node is lead here?
   (https://github.com/apache/kafka/blob/0f7cd4dcdeb2c705c01743927e36b66b06010f20/raft/src/main/java/org/apache/kafka/raft/RaftClient.java#L150



-- 
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