dajac commented on code in PR #12029:
URL: https://github.com/apache/kafka/pull/12029#discussion_r860897319
##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -674,21 +682,29 @@ class UnifiedLog(@volatile var logStartOffset: Long,
}
/**
- * Rename the directory of the local log
+ * Rename the directory of the local log. If the log's directory is being
renamed for async deletion due to a
+ * StopReplica request, then the shouldReinitialize parameter should be set
to false, otherwise it should be set to true.
*
+ * @param name The new name that this log's directory is being renamed to
+ * @param shouldReinitialize Whether the log's metadata should be
reinitialized after renaming
* @throws KafkaStorageException if rename fails
*/
- def renameDir(name: String): Unit = {
+ def renameDir(name: String, shouldReinitialize: Boolean): Unit = {
lock synchronized {
maybeHandleIOException(s"Error while renaming dir for $topicPartition in
log dir ${dir.getParent}") {
// Flush partitionMetadata file before initializing again
maybeFlushMetadataFile()
Review Comment:
It is not orthogonal in my opinion.
`testStopReplicaWithDeletePartitionAndExistingPartitionAndNewerLeaderEpochAndIOException`
was meant to ensure `IOException` is handled correctly on the stop replica
path. The issue is that the change made is this PR makes the trigger used
obsolete.
It seems that we could just replace the following block in
`testStopReplicaWithExistingPartition`:
```
if (throwIOException) {
// Delete the underlying directory to trigger an KafkaStorageException
val dir = partition.log.get.dir
Utils.delete(dir)
dir.createNewFile()
}
```
by the following one:
```
if (throwIOException) {
// Replace underlying PartitionMetadataFile with a mock which thrown
// a KafkaStorageException when maybeFlush is called.
val mockPartitionMetadataFile = mock(classOf[PartitionMetadataFile])
when(mockPartitionMetadataFile.maybeFlush()).thenThrow(new
KafkaStorageException())
partition.log.get.partitionMetadataFile = Some(mockPartitionMetadataFile)
}
```
and we are good to go. 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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
