[bookkeeper] branch master updated: Fix ByteBuf leaking at Journal

[si...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 61ed8a6  Fix ByteBuf leaking at Journal
61ed8a6 is described below

commit 61ed8a697e9c1f6edbd8996819f602f6ca79920d
Author: Sijie Guo <si...@apache.org>
AuthorDate: Mon Mar 19 13:13:37 2018 -0700

    Fix ByteBuf leaking at Journal
    
    Descriptions of the changes in this PR:
    
    BufferLogChannel uses a pooled bytebuf for buffering writes. However Journal only closes the underlying log file channel, but it doesn't close buffer log channel, which can cause bytebuf leaking.
    
    This problem get exposed at #1268
    
    Author: Sijie Guo <si...@apache.org>
    
    Reviewers: Enrico Olivelli <eo...@gmail.com>, Matteo Merli <mm...@apache.org>
    
    This closes #1275 from sijie/fix_bytebuf_leaking_on_journal
---
 .../src/main/java/org/apache/bookkeeper/bookie/Journal.java      | 9 ++++++---
 .../org/apache/bookkeeper/test/BookieJournalRollingTest.java     | 4 ++++
 2 files changed, 10 insertions(+), 3 deletions(-)

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 3d79acb..94aafad 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 class Journal extends BookieCriticalThread implements CheckpointSource {
         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 class Journal extends BookieCriticalThread implements CheckpointSource {
             // 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 class Journal extends BookieCriticalThread implements CheckpointSource {
                             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 class Journal extends BookieCriticalThread implements CheckpointSource {
                 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 class Journal extends BookieCriticalThread implements CheckpointSource {
             // 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 0c7c492..83d7ba1 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 @@ public class BookieJournalRollingTest extends BookKeeperClusterTestCase {
                 end = start + read - 1;
             }
         }
+
+        for (LedgerHandle lh : lhs) {
+            lh.close();
+        }
     }
 
     /**

-- 
To stop receiving notification emails like this one, please contact
sijie@apache.org.