[avro] branch master updated: AVRO-3033: Fix flaky file descriptor test (#1078)

[rs...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

rskraba pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new 568ee67  AVRO-3033: Fix flaky file descriptor test (#1078)
568ee67 is described below

commit 568ee67c2d0c8d62049741ae78dc91910aa1b6f2
Author: RyanSkraba <ry...@skraba.com>
AuthorDate: Tue Feb 2 13:08:15 2021 +0100

    AVRO-3033: Fix flaky file descriptor test (#1078)
    
    It looks like before fixing the bug AVRO-2286, the number of openFilesBeforeOperation was always greater than openFilesAfterOperation. In practice, now they're usually the same value, which is promising evidence that there isn't a resource leak there.
    
    The flakiness of the test comes from the fact that "something" might be fuzzing either of them off by one. I used to see this in travis and local builds too (but infrequently). It might be a background, system-process or an action like garbage collection. I don't think it's instability of the machine, it's just not as deterministic as we'd like.
---
 .../java/org/apache/avro/TestDataFileReader.java   | 40 +++++++++++++---------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java b/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java
index c222685..155ce6b 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestDataFileReader.java
@@ -18,7 +18,6 @@
 package org.apache.avro;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.File;
@@ -45,23 +44,32 @@ public class TestDataFileReader {
   @Test
   // regression test for bug AVRO-2286
   public void testForLeakingFileDescriptors() throws IOException {
-    Path emptyFile = Files.createTempFile("empty", ".avro");
-    Files.deleteIfExists(emptyFile);
-    Files.createFile(emptyFile);
-
-    long openFilesBeforeOperation = getNumberOfOpenFileDescriptors();
-    try (DataFileReader<Object> reader = new DataFileReader<>(emptyFile.toFile(), new GenericDatumReader<>())) {
-      fail("Reading on empty file is supposed to fail.");
-    } catch (IOException e) {
-      // everything going as supposed to
-    }
-    Files.delete(emptyFile);
+    StringBuilder sb = new StringBuilder();
+    int maxTries = 3;
+    for (int tries = 0; tries < maxTries; tries++) {
+      Path emptyFile = Files.createTempFile("empty", ".avro");
+      Files.deleteIfExists(emptyFile);
+      Files.createFile(emptyFile);
+
+      long openFilesBeforeOperation = getNumberOfOpenFileDescriptors();
+      try (DataFileReader<Object> reader = new DataFileReader<>(emptyFile.toFile(), new GenericDatumReader<>())) {
+        fail("Reading on empty file is supposed to fail.");
+      } catch (IOException e) {
+        // everything going as supposed to
+      }
+      Files.delete(emptyFile);
 
-    long openFilesAfterOperation = getNumberOfOpenFileDescriptors();
+      long openFilesAfterOperation = getNumberOfOpenFileDescriptors();
+      if (openFilesBeforeOperation == openFilesAfterOperation)
+        return;
 
-    // Sometimes we have less open files because of a GC run during the test cycle.
-    assertTrue("File descriptor leaked from new DataFileReader() (expected:" + openFilesBeforeOperation + " actual:"
-        + openFilesAfterOperation + ")", openFilesBeforeOperation >= openFilesAfterOperation);
+      // Sometimes the number of file descriptors is off due to other processes or
+      // garbage
+      // collection. We note each inconsistency and retry.
+      sb.append(openFilesBeforeOperation).append("!=").append(openFilesAfterOperation).append(",");
+    }
+    fail("File descriptor leaked from new DataFileReader() over " + maxTries + " tries: ("
+        + sb.substring(0, sb.length() - 1) + ")");
   }
 
   private long getNumberOfOpenFileDescriptors() {