junrao commented on code in PR #20289:
URL: https://github.com/apache/kafka/pull/20289#discussion_r2743823886
##########
core/src/main/scala/kafka/log/LogManager.scala:
##########
@@ -1247,9 +1247,13 @@ class LogManager(logDirs: Seq[File],
try {
sourceLog.foreach { srcLog =>
srcLog.renameDir(UnifiedLog.logDeleteDirName(topicPartition), true)
- // Now that replica in source log directory has been successfully
renamed for deletion.
- // Close the log, update checkpoint files, and enqueue this log to be
deleted.
- srcLog.close()
+ // Now that replica in source log directory has been successfully
renamed for deletion,
+ // update checkpoint files and enqueue this log to be deleted.
+ // Note: We intentionally do NOT close the log here to avoid race
conditions where concurrent
+ // operations (e.g., log flusher, fetch requests) might encounter
ClosedChannelException.
+ // The log will be deleted asynchronously by the background
delete-logs thread.
+ // File handles are intentionally left open; Unix semantics allow the
renamed files
+ // to remain accessible until all handles are closed.
Review Comment:
> //The log will be deleted asynchronously by the background
delete-logs thread.
> // File handles are intentionally left open; Unix semantics allow
the renamed files
> // to remain accessible until all handles are closed.
How about the following?
File handles are intentionally left open; Unix semantics allow the renamed
files
to remain accessible until all handles are closed.
The log will be deleted asynchronously by the background delete-logs thread.
File handles are closed and files are deleted after a configured delay
log.segment.delete.delay.ms.
At that time, the expectation is that no other concurrent operations need to
access
the deleted file handles any more.
--
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]
