ableegoldman commented on a change in pull request #10608:
URL: https://github.com/apache/kafka/pull/10608#discussion_r622719786



##########
File path: 
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateDirectory.java
##########
@@ -431,7 +397,7 @@ private void cleanRemovedTasksCalledByCleanerThread(final 
long cleanupDelayMs) {
                         if (now > lastModifiedMs + cleanupDelayMs) {
                             log.info("{} Deleting obsolete state directory {} 
for task {} as {}ms has elapsed (cleanup delay is {}ms).",
                                 logPrefix(), dirName, id, now - 
lastModifiedMs, cleanupDelayMs);
-                            Utils.delete(taskDir, 
Collections.singletonList(new File(taskDir, LOCK_FILE_NAME)));

Review comment:
       This change seems innocuous but has a pretty sweet side effect: because 
we had to exclude the lock file during the deletion (due to some stupid Windows 
bug 🙄 ), this utility method was not actually deleting the task directory 
itself. AFAICT this means we were never deleting these empty directories at any 
point, unless the user manually called KafkaStreams#cleanUp. It's not a 
horrible bug, but it's just obnoxious so I'm glad we can finally be rid of the 
Windows ball & chain that was its `DirectoryNotEmptyException`




-- 
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to