KAFKA-13391: don't fsync directory on Windows OS #11426
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ccding
approved these changes
Oct 22, 2021
| @@ -948,10 +948,12 @@ public static void atomicMoveWithFallback(Path source, Path target, boolean need | |||
| /** | |||
| * Flushes dirty directories to guarantee crash consistency. | |||
| * | |||
| * Note: We don't fsync directory on Windows OS because it'll throw AccessDeniedException (KAFKA-13391) | |||
There was a problem hiding this comment.
nit: directory -> directories, because -> because otherwise
There was a problem hiding this comment.
Thanks for the comment. Updated.
junrao
pushed a commit
that referenced
this pull request
Oct 25, 2021
Reviewers: Cong Ding <cong@ccding.com>, Jun Rao <junrao@gmail.com>
AndyGee
reviewed
Dec 13, 2021
| * @throws IOException if flushing the directory fails. | ||
| */ | ||
| public static void flushDir(Path path) throws IOException { | ||
| if (path != null) { | ||
| if (path != null && !OperatingSystem.IS_WINDOWS) { | ||
| try (FileChannel dir = FileChannel.open(path, StandardOpenOption.READ)) { | ||
| dir.force(true); |
There was a problem hiding this comment.
There is a file lock on this file that causes the issue, which might be hiding another issue even on other platforms.
xdgrulez
pushed a commit
to xdgrulez/kafka
that referenced
this pull request
Dec 22, 2021
Reviewers: Cong Ding <cong@ccding.com>, Jun Rao <junrao@gmail.com>
|
When will this PR will be released in the 3.0.1/X releases? |
|
@mosesonline , here's the release plan for v3.0.1. FYI |
lmr3796
pushed a commit
to lmr3796/kafka
that referenced
this pull request
Jun 2, 2022
Reviewers: Cong Ding <cong@ccding.com>, Jun Rao <junrao@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #10680, we added
fysncon dir to maintain crash consistency. But, it looks like Windows OS doesn't supportfsyncon directory. The same issues also happen on LUCENE and HDFS projects. And the way they fix it is pretty much the same: to skip fsync directory on Windows OS. Here are the patches for both LUCENE-5588 and HDFS-13586.I followed their way to fix this issue. No tests added since it's just an OS check added, no logic change.
Committer Checklist (excluded from commit message)