paulirwin commented on code in PR #938:
URL: https://github.com/apache/lucenenet/pull/938#discussion_r1597688147
##########
src/Lucene.Net/Util/IOUtils.cs:
##########
@@ -516,8 +519,53 @@ public static void ReThrowUnchecked(Exception th)
}
}
- // LUCENENET specific: Fsync is pointless in .NET, since we are
- // calling FileStream.Flush(true) before the stream is disposed
- // which means we never need it at the point in Java where it is
called.
+ public static void Fsync(FileSystemInfo fileToSync, bool isDir)
+ {
+ // LUCENENET NOTE: there is a bug in 4.8 where it tries to fsync a
directory on Windows,
+ // which is not supported in OpenJDK. This change adopts the
latest Lucene code as of 9.10
+ // and only fsyncs directories on Linux and macOS.
+
+ // If the file is a directory we have to open read-only, for
regular files we must open r/w for
+ // the fsync to have an effect.
+ // See
http://blog.httrack.com/blog/2013/11/15/everything-you-always-wanted-to-know-about-fsync/
+ if (isDir && Constants.WINDOWS)
+ {
+ // opening a directory on Windows fails, directories can not
be fsynced there
+ if (System.IO.Directory.Exists(fileToSync.FullName) == false)
+ {
+ // yet do not suppress trying to fsync directories that do
not exist
+ throw new DirectoryNotFoundException($"Directory does not
exist: {fileToSync}");
+ }
+ return;
+ }
+
+ try
+ {
+ // LUCENENET specific: was: file.force(true);
+ // We must call fsync on the parent directory, requiring some
custom P/Invoking
+ if (Constants.WINDOWS)
+ {
+ WindowsFsyncSupport.Fsync(fileToSync.FullName, isDir);
+ }
+ else
+ {
+ PosixFsyncSupport.Fsync(fileToSync.FullName, isDir);
+ }
+ }
+ catch (Exception e) when (e.IsIOException())
+ {
+ if (isDir)
+ {
+ Debug.Assert((Constants.LINUX || Constants.MAC_OS_X) ==
false,
Review Comment:
Are you sure we should use `Debugging`? `Debug.Assert` will not be emitted
in Release builds, while AFAICT `Debugging` calls would because they are not
`[Conditional("DEBUG")]`. There is no production concern (nor string
concatenation concern) with `Debug.Assert`. That said, I'd almost rather remove
the line entirely because we already check for Windows now above.
--
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]
