From 24bc7b1edd6780915b5592a5a45a966222dd2bb4 Mon Sep 17 00:00:00 2001 From: Uwe Schindler Date: Sun, 22 Nov 2015 10:27:57 +0000 Subject: [PATCH] LUCENE-6902: Don't retry to fsync files / directories; fail immediately git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1715617 13f79535-47bb-0310-9956-ffa450edef68 --- lucene/CHANGES.txt | 3 ++ .../java/org/apache/lucene/util/IOUtils.java | 44 +++++-------------- .../org/apache/lucene/util/TestIOUtils.java | 24 ++++++++++ 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index cf1ae74758c..9c2e886937d 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -372,6 +372,9 @@ Other * LUCENE-6893: factor out CorePlusQueriesParser from CorePlusExtensionsParser (Christine Poerschke) +* LUCENE-6902: Don't retry to fsync files / directories; fail + immediately. (Daniel Mitterdorfer, Uwe Schindler) + Build * LUCENE-6732: Improve checker for invalid source patterns to also diff --git a/lucene/core/src/java/org/apache/lucene/util/IOUtils.java b/lucene/core/src/java/org/apache/lucene/util/IOUtils.java index 03712880d70..c6d777f0fa7 100644 --- a/lucene/core/src/java/org/apache/lucene/util/IOUtils.java +++ b/lucene/core/src/java/org/apache/lucene/util/IOUtils.java @@ -414,46 +414,22 @@ public final class IOUtils { * because not all file systems and operating systems allow to fsync on a directory) */ public static void fsync(Path fileToSync, boolean isDir) throws IOException { - IOException exc = null; - // 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/ try (final FileChannel file = FileChannel.open(fileToSync, isDir ? StandardOpenOption.READ : StandardOpenOption.WRITE)) { - for (int retry = 0; retry < 5; retry++) { - try { - file.force(true); - return; - } catch (IOException ioe) { - if (exc == null) { - exc = ioe; - } - try { - // Pause 5 msec - Thread.sleep(5L); - } catch (InterruptedException ie) { - ThreadInterruptedException ex = new ThreadInterruptedException(ie); - ex.addSuppressed(exc); - throw ex; - } - } - } + file.force(true); } catch (IOException ioe) { - if (exc == null) { - exc = ioe; + if (isDir) { + // TODO: LUCENE-6169 - Fix this assert once Java 9 problems are solved! + assert (Constants.LINUX || Constants.MAC_OS_X) == false || Constants.JRE_IS_MINIMUM_JAVA9 : + "On Linux and MacOSX fsyncing a directory should not throw IOException, "+ + "we just don't want to rely on that in production (undocumented). Got: " + ioe; + // Ignore exception if it is a directory + return; } + // Throw original exception + throw ioe; } - - if (isDir) { - // TODO: LUCENE-6169 - Fix this assert once Java 9 problems are solved! - assert (Constants.LINUX || Constants.MAC_OS_X) == false || Constants.JRE_IS_MINIMUM_JAVA9 : - "On Linux and MacOSX fsyncing a directory should not throw IOException, "+ - "we just don't want to rely on that in production (undocumented). Got: " + exc; - // Ignore exception if it is a directory - return; - } - - // Throw original exception - throw exc; } /** If the dir is an {@link FSDirectory} or wraps one via possibly diff --git a/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java b/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java index 848e009aefe..7a112cd2f65 100644 --- a/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java +++ b/lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java @@ -466,4 +466,28 @@ public class TestIOUtils extends LuceneTestCase { assertFalse(IOUtils.spinsLinux(mockPath)); } + public void testFsyncDirectory() throws Exception { + Path dir = createTempDir(); + dir = FilterPath.unwrap(dir).toRealPath(); + + Path devdir = dir.resolve("dev"); + Files.createDirectories(devdir); + IOUtils.fsync(devdir, true); + // no exception + } + + public void testFsyncFile() throws Exception { + Path dir = createTempDir(); + dir = FilterPath.unwrap(dir).toRealPath(); + + Path devdir = dir.resolve("dev"); + Files.createDirectories(devdir); + Path somefile = devdir.resolve("somefile"); + try (OutputStream o = Files.newOutputStream(somefile)) { + o.write("0\n".getBytes(StandardCharsets.US_ASCII)); + } + IOUtils.fsync(somefile, false); + // no exception + } + }