junrao commented on code in PR #12029:
URL: https://github.com/apache/kafka/pull/12029#discussion_r851543064
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2615,7 +2616,13 @@ class ReplicaManagerTest {
@Test
def
testStopReplicaWithDeletePartitionAndExistingPartitionAndNewerLeaderEpochAndIOException():
Unit = {
- testStopReplicaWithExistingPartition(2, true, true,
Errors.KAFKA_STORAGE_ERROR)
+ // 1. Even though we are trying to trigger an IOException by deleting the
underlying log directory,
+ // the foreground deletion of a replica no longer reads from the
underlying directory, and thus
+ // should trigger no exceptions. It means the stopReplica operation
running the foreground
+ // deletion should be able to finish without any errors.
+ // 2. During the background deletion, a FileSystemException will be
triggered when we try
+ // to delete the log segments, which is then converted to a
KafkaStorageException.
+ testStopReplicaWithExistingPartition(2, true, true, Errors.NONE, true)
Review Comment:
Since all tests now set expectedOutput to NONE, could we just remove that
param?
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2703,6 +2711,26 @@ class ReplicaManagerTest {
assertEquals(HostedPartition.None, replicaManager.getPartition(tp0))
assertFalse(readRecoveryPointCheckpoint().contains(tp0))
assertFalse(readLogStartOffsetCheckpoint().contains(tp0))
+
+ // verify that there is log to be deleted
+ val logsToBeDeleted = replicaManager.logManager.logsToBeDeleted
Review Comment:
This seems an indirect way of testing the logic. logsToBeDeleted will be
picked up by the scheduler in LogManager, which uses mockTime. Could we just
advance the mock time to trigger the scheduler?
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
