Remove Assertion on fsync Dir IOE

[original-brownbear]

• This assertion is not correct. fsync might not throw on OSX and Linux, but you also get this IOException if you try to open a directory whose parent does not exist for example (or whatever other IOException you might get from opening a directory).

I think we also have the same assertion in Lucene, I'd open a PR for that too if this one is deemed ok :)

[elasticmachine]

Pinging @elastic/es-core-infra

[jasontedor]

I don't think this is the right change. Rather, I think that we want to move this logic to be inside the try-with-resources block, so that we aren't being lenient on failing to fsync the directory if we, for example, fail to open the directory because it does not exist or the disk is crap. Today, we are lenient in that regard and that looks dangerous to me. So rather, I think we should change along the lines of:

diff --git a/libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java b/libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java
index 46d19d2a814..f82070183e0 100644
--- a/libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java
+++ b/libs/core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java
@@ -264,17 +264,17 @@ public final class IOUtils {
      */
     public static void fsync(final Path fileToSync, final boolean isDir) throws IOException {
         try (FileChannel file = FileChannel.open(fileToSync, isDir ? StandardOpenOption.READ : StandardOpenOption.WRITE)) {
-            file.force(true);
-        } catch (final IOException ioe) {
-            if (isDir) {
-                assert (LINUX || MAC_OS_X) == false :
-                        "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;
+            try {
+                file.force(true);
+            } catch (final IOException e) {
+                if (isDir) {
+                    assert (LINUX || MAC_OS_X) == false :
+                            "on Linux and MacOSX fsyncing a directory should not throw IOException, "+
+                                    "we just don't want to rely on that in production (undocumented); got: " + e;
+                    return;
+                }
+                throw e;
             }
-            // throw original exception
-            throw ioe;
         }
     }
 }

Then, I'm not sure that we need to drop the assertion.

[jasontedor]

Because there is an active test failure caused by this issue in #42950, I opened #42972 with my suggestion so that we can close out test failures quickly. Then we can continue to discuss here whether or not to drop the assertion.

[original-brownbear]

@jasontedor as I said in your PR, I agree your solution makes more sense.