FrankYang0529 commented on PR #15616: URL: https://github.com/apache/kafka/pull/15616#issuecomment-2067936507
> This PR is good but it seems to me `LogSegment` should NOT guess the directory structure managed by upper class (i.e `LogManager`). > > It seems the root cause is caused by following steps: > > 1. the segments to be deleted removed from `LocalLog` > 2. `LocalLog#renameDir` move whole folder > 3. `LocalLog#renameDir` update the parent folder for all segments. However, the segments to be deleted are removed form inner collection already. Hence, some `Segment#log` has a stale file. > > If I understand correctly, another solution is that we pass a function to get latest dir when calling `deleteSegmentFiles` ( > > https://github.com/apache/kafka/blob/2d4abb85bf4a3afb1e3359a05786ab8f3fda127e/core/src/main/scala/kafka/log/LocalLog.scala#L904 > > ). If deleting segment get not-found error, we call `updateParentDir` and delete it again. > WDYT? Hi @chia7712, thanks for the great suggestion. I took a look `LogSegment#deleteIfExists` and `LogSegment#deleteTypeIfExists`. If we want to handle fallback deletion in `LocalLog`, we may need to return true/false in that two functions. However, `LogSegment#deleteIfExists` uses `Utils.tryAll` to handle 4 try/catch blocks. If we want to return true/false, we need to refactor `Utils.tryAll` as well. Finally, `LogSegment#deleteIfExists` is not only used by `LocalLog.deleteSegmentFiles`, but also `LocalLog.splitOverflowedSegment`. We need to handle `LocalLog.splitOverflowedSegment` path, too. I think we can use another Jira to track the change. Thanks. -- 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
