Explicitly release resources in LogFileReader and TestHoodieClientBase

hoodie-common/src/main/java/com/uber/hoodie/common/table/log/AbstractHoodieLogRecordScanner.java

@@ -30,6 +30,7 @@ import com.uber.hoodie.common.table.log.block.HoodieDeleteBlock;
 import com.uber.hoodie.common.table.log.block.HoodieLogBlock;
 import com.uber.hoodie.common.util.SpillableMapUtils;
 import com.uber.hoodie.exception.HoodieIOException;
+import java.io.IOException;
 import java.util.ArrayDeque;
 import java.util.Arrays;
 import java.util.Deque;
@@ -115,9 +116,10 @@ public abstract class AbstractHoodieLogRecordScanner {
    * Scan Log files
    */
   public void scan() {
+    HoodieLogFormatReader logFormatReaderWrapper = null;
     try {
       // iterate over the paths
-      HoodieLogFormatReader logFormatReaderWrapper =
+      logFormatReaderWrapper =
           new HoodieLogFormatReader(fs,
               logFilePaths.stream().map(logFile -> new HoodieLogFile(new Path(logFile)))
                   .collect(Collectors.toList()), readerSchema, readBlocksLazily, reverseReader, bufferSize);
@@ -239,6 +241,15 @@ public abstract class AbstractHoodieLogRecordScanner {
     } catch (Exception e) {
       log.error("Got exception when reading log file", e);
       throw new HoodieIOException("IOException when reading log file ");
+    } finally {
+      try {
+        if (null != logFormatReaderWrapper) {
+          logFormatReaderWrapper.close();
+        }
+      } catch (IOException ioe) {
+        // Eat exception as we do not want to mask the original exception that can happen
+        log.error("Unable to close log format reader", ioe);
+      }
     }
   }
 

hoodie-common/src/main/java/com/uber/hoodie/common/table/log/HoodieLogFileReader.java

@@ -62,6 +62,7 @@ class HoodieLogFileReader implements HoodieLogFormat.Reader {
   private long reverseLogFilePosition;
   private long lastReverseLogFilePosition;
   private boolean reverseReader;
+  private boolean closed = false;
 
   HoodieLogFileReader(FileSystem fs, HoodieLogFile logFile, Schema readerSchema, int bufferSize,
       boolean readBlockLazily, boolean reverseReader) throws IOException {
@@ -95,13 +96,13 @@ class HoodieLogFileReader implements HoodieLogFormat.Reader {
   }
 
   /**
-   * Close the inputstream when the JVM exits
+   * Close the inputstream if not closed when the JVM exits
    */
   private void addShutDownHook() {
     Runtime.getRuntime().addShutdownHook(new Thread() {
       public void run() {
         try {
-          inputStream.close();
+          close();
         } catch (Exception e) {
           log.warn("unable to close input stream for log file " + logFile, e);
           // fail silently for any sort of exception
@@ -277,7 +278,10 @@ class HoodieLogFileReader implements HoodieLogFormat.Reader {
 
   @Override
   public void close() throws IOException {
-    this.inputStream.close();
+    if (!closed) {
+      this.inputStream.close();
+      closed = true;
+    }
   }
 
   @Override

hoodie-common/src/main/java/com/uber/hoodie/common/table/log/HoodieLogFormatReader.java

@@ -20,6 +20,7 @@ import com.uber.hoodie.common.model.HoodieLogFile;
 import com.uber.hoodie.common.table.log.block.HoodieLogBlock;
 import com.uber.hoodie.exception.HoodieIOException;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
 import org.apache.avro.Schema;
 import org.apache.hadoop.fs.FileSystem;
@@ -29,6 +30,8 @@ import org.apache.log4j.Logger;
 public class HoodieLogFormatReader implements HoodieLogFormat.Reader {
 
   private final List<HoodieLogFile> logFiles;
+  // Readers for previously scanned log-files that are still open
+  private final List<HoodieLogFileReader> prevReadersInOpenState;
   private HoodieLogFileReader currentReader;
   private final FileSystem fs;
   private final Schema readerSchema;
@@ -46,6 +49,7 @@ public class HoodieLogFormatReader implements HoodieLogFormat.Reader {
     this.readBlocksLazily = readBlocksLazily;
     this.reverseLogReader = reverseLogReader;
     this.bufferSize = bufferSize;
+    this.prevReadersInOpenState = new ArrayList<>();
     if (logFiles.size() > 0) {
       HoodieLogFile nextLogFile = logFiles.remove(0);
       this.currentReader = new HoodieLogFileReader(fs, nextLogFile, readerSchema, bufferSize, readBlocksLazily, false);
@@ -53,7 +57,20 @@ public class HoodieLogFormatReader implements HoodieLogFormat.Reader {
   }
 
   @Override
+  /**
+   * Note : In lazy mode, clients must ensure close() should be called only after processing
+   * all log-blocks as the underlying inputstream will be closed.
+   * TODO: We can introduce invalidate() API at HoodieLogBlock and this object can call invalidate on
+   * all returned log-blocks so that we check this scenario specifically in HoodieLogBlock
+   */
   public void close() throws IOException {
+
+    for (HoodieLogFileReader reader : prevReadersInOpenState) {
+      reader.close();
+    }
+
+    prevReadersInOpenState.clear();
+
     if (currentReader != null) {
       currentReader.close();
     }
@@ -69,6 +86,12 @@ public class HoodieLogFormatReader implements HoodieLogFormat.Reader {
     } else if (logFiles.size() > 0) {
       try {
         HoodieLogFile nextLogFile = logFiles.remove(0);
+        // First close previous reader only if readBlockLazily is true
+        if (!readBlocksLazily) {
+          this.currentReader.close();
+        } else {
+          this.prevReadersInOpenState.add(currentReader);
+        }
         this.currentReader = new HoodieLogFileReader(fs, nextLogFile, readerSchema, bufferSize, readBlocksLazily,
             false);
       } catch (IOException io) {

hoodie-common/src/test/java/com/uber/hoodie/common/table/log/HoodieLogFormatTest.java

@@ -304,6 +304,7 @@ public class HoodieLogFormatTest {
         dataBlockRead.getRecords().size());
     assertEquals("Both records lists should be the same. (ordering guaranteed)", copyOfRecords,
         dataBlockRead.getRecords());
+    reader.close();
   }
 
   @SuppressWarnings("unchecked")
@@ -370,6 +371,7 @@ public class HoodieLogFormatTest {
         dataBlockRead.getRecords().size());
     assertEquals("Both records lists should be the same. (ordering guaranteed)", copyOfRecords3,
         dataBlockRead.getRecords());
+    reader.close();
   }
 
   @SuppressWarnings("unchecked")
@@ -454,6 +456,8 @@ public class HoodieLogFormatTest {
     //assertEquals("", "something-random", new String(corruptBlock.getCorruptedBytes()));
     assertFalse("There should be no more block left", reader.hasNext());
 
+    reader.close();
+
     // Simulate another failure back to back
     outputStream = fs.append(writer.getLogFile().getPath());
     // create a block with
@@ -493,6 +497,7 @@ public class HoodieLogFormatTest {
     assertTrue("We should get the last block next", reader.hasNext());
     reader.next();
     assertFalse("We should have no more blocks left", reader.hasNext());
+    reader.close();
   }
 
 
@@ -1097,7 +1102,7 @@ public class HoodieLogFormatTest {
     assertEquals(block.getBlockType(), HoodieLogBlockType.AVRO_DATA_BLOCK);
     dBlock = (HoodieAvroDataBlock) block;
     assertEquals(dBlock.getRecords().size(), 100);
-
+    reader.close();
   }
 
   @SuppressWarnings("unchecked")
@@ -1167,6 +1172,7 @@ public class HoodieLogFormatTest {
         dataBlockRead.getRecords());
 
     assertFalse(reader.hasPrev());
+    reader.close();
   }
 
   @Test
@@ -1224,6 +1230,7 @@ public class HoodieLogFormatTest {
       e.printStackTrace();
       // We should have corrupted block
     }
+    reader.close();
   }
 
   @SuppressWarnings("unchecked")
@@ -1283,5 +1290,6 @@ public class HoodieLogFormatTest {
         dataBlockRead.getRecords());
 
     assertFalse(reader.hasPrev());
+    reader.close();
   }
 }