[GitHub] sijie closed pull request #1275: Fix ByteBuf leaking at Journal

[GitBox <gi...@apache.org>]

sijie closed pull request #1275: Fix ByteBuf leaking at Journal
URL: https://github.com/apache/bookkeeper/pull/1275
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
index 3d79acb59..94aafad18 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
@@ -904,6 +904,7 @@ public void run() {
         paddingBuff.writeZero(paddingBuff.capacity());
         final int journalFormatVersionToWrite = conf.getJournalFormatVersionToWrite();
         final int journalAlignmentSize = conf.getJournalAlignmentSize();
+        BufferedChannel bc = null;
         JournalChannel logFile = null;
         forceWriteThread.start();
         Stopwatch journalCreationWatcher = Stopwatch.createUnstarted();
@@ -915,7 +916,6 @@ public void run() {
             // could only be used to measure elapsed time.
             // http://docs.oracle.com/javase/1.5.0/docs/api/java/lang/System.html#nanoTime%28%29
             long logId = journalIds.isEmpty() ? System.currentTimeMillis() : journalIds.get(journalIds.size() - 1);
-            BufferedChannel bc = null;
             long lastFlushPosition = 0;
             boolean groupWhenTimeout = false;
 
@@ -1053,6 +1053,10 @@ public void run() {
                             batchSize = 0L;
                             // check whether journal file is over file limit
                             if (shouldRolloverJournal) {
+                                // if the journal file is rolled over, the journal file will be closed after last
+                                // entry is force written to disk. the `bc` is not used anymore, so close it to release
+                                // the buffers in `bc`.
+                                IOUtils.close(LOG, bc);
                                 logFile = null;
                                 continue;
                             }
@@ -1089,8 +1093,6 @@ public void run() {
                 numEntriesToFlush++;
                 qe = null;
             }
-            logFile.close();
-            logFile = null;
         } catch (IOException ioe) {
             LOG.error("I/O exception in Journal thread!", ioe);
         } catch (InterruptedException ie) {
@@ -1102,6 +1104,7 @@ public void run() {
             // the bookie. If we execute this as a part of graceful shutdown,
             // close will flush the file system cache making any previous
             // cached writes durable so this is fine as well.
+            IOUtils.close(LOG, bc);
             IOUtils.close(LOG, logFile);
         }
         LOG.info("Journal exited loop!");
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieJournalRollingTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieJournalRollingTest.java
index 0c7c49282..83d7ba174 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieJournalRollingTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieJournalRollingTest.java
@@ -160,6 +160,10 @@ private void validLedgerEntries(long[] ledgerIds, int msgSize, int numMsgs) thro
                 end = start + read - 1;
             }
         }
+
+        for (LedgerHandle lh : lhs) {
+            lh.close();
+        }
     }
 
     /**


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services