dajac commented on code in PR #12029:
URL: https://github.com/apache/kafka/pull/12029#discussion_r852680173
##########
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
+ assertEquals(1, logsToBeDeleted.size())
+ val (removedLog, _) = logsToBeDeleted.take()
+
assertFalse(removedLog.logDirFailureChannel.hasOfflineLogDir(logDir.toString))
+
+ // trigger the background deletion
+ var kafkaStorageExceptionCaptured = false
+ try {
+ removedLog.delete()
+ } catch {
+ case _: KafkaStorageException =>
+ kafkaStorageExceptionCaptured = true
+ }
+ assertEquals(expectedExceptionInBackgroundDeletion,
kafkaStorageExceptionCaptured)
Review Comment:
I wonder if this test is the appropriate place for this check because it
does not really test the stop replica path. Would it make sense to have it in a
separate test? @junrao What do you think?
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -2615,7 +2615,10 @@ class ReplicaManagerTest {
@Test
def
testStopReplicaWithDeletePartitionAndExistingPartitionAndNewerLeaderEpochAndIOException():
Unit = {
Review Comment:
@gitlw Could `maybeFlushMetadataFile` in `renameDir` still throw an
`IOException`? The test does not force this condition at the moment. Do we want
to gate `maybeFlushMetadataFile` as well or do we need to update the test to
trigger it?
--
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
