LB-Yu commented on code in PR #1446:
URL: https://github.com/apache/fluss/pull/1446#discussion_r2256095801
##########
fluss-server/src/test/java/com/alibaba/fluss/server/zk/ZooKeeperClientTest.java:
##########
@@ -220,6 +220,31 @@ void testBatchCreateLeaderAndIsr() throws Exception {
}
}
+ @Test
+ void testBatchUpdateLeaderAndIsr() throws Exception {
+ int totalCount = 100;
+
+ // try to register bucket leaderAndIsr
+ Map<TableBucket, LeaderAndIsr> leaderAndIsrList = new HashMap<>();
+ for (int i = 0; i < totalCount; i++) {
+ TableBucket tableBucket = new TableBucket(1, i);
+ LeaderAndIsr leaderAndIsr =
+ new LeaderAndIsr(i, 10, Arrays.asList(i + 1, i + 2, i +
3), 100, 1000);
+ leaderAndIsrList.put(tableBucket, leaderAndIsr);
+ zookeeperClient.registerLeaderAndIsr(tableBucket, leaderAndIsr);
+ }
Review Comment:
Thank you for pointing out the issue here. However, we do need to call
`register leaderAndIsr` first, otherwise there will be a "node does not exist"
problem when updating. In the previous implementation, I didn't use the new
`LeaderAndIsr` to test `batchUpdateLeaderAndIsr`, which resulted in the test
passing even after deletion
(`zookeeperClient.batchUpdateLeaderAndIsr(updateLeaderAndIsrList);`). Now I am
using a new `LeaderAndIsr` to verify that `batchUpdateLeaderAndIsr` is indeed
updated successfully.
##########
fluss-server/src/test/java/com/alibaba/fluss/server/coordinator/CoordinatorEventProcessorTest.java:
##########
@@ -761,6 +766,59 @@ void testNotifyOffsetsWithShrinkISR(@TempDir Path tempDir)
throws Exception {
verifyReceiveRequestExceptFor(3, leader,
NotifyKvSnapshotOffsetRequest.class);
}
+ @Test
+ void testProcessAdjustIsr() throws Exception {
Review Comment:
I don't think this test is invalid. When handling the
`AdjustIsrReceivedEvent` event (in the
`CoordinatorEventProcessor#tryProcessAdjustIsr` method), the `bucketEpoch` of
LeaderAndIsr is incremented (for example, from 100 to 101). This test verifies
from the context that the `bucketEpoch` has indeed increased.
The main purpose of this test is to verify whether the handling of the
`AdjustIsrReceivedEvent` event is correct. I think we shouldn't modify
`LeaderAndIsr` manually, but rather check whether `LeaderAndIsr` reaches the
expected state after `AdjustIsrReceivedEvent` is properly handled. What do you
think?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
