You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/10/09 00:53:29 UTC

[bookkeeper] branch branch-4.7 updated: ISSUE #1737: EntryMemTable.newEntry: always make a copy

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

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


The following commit(s) were added to refs/heads/branch-4.7 by this push:
     new fef7f00  ISSUE #1737: EntryMemTable.newEntry: always make a copy
fef7f00 is described below

commit fef7f006fcf0ad20516e5ebeb732d690af044ec1
Author: Samuel Just <sj...@salesforce.com>
AuthorDate: Mon Oct 8 17:53:24 2018 -0700

    ISSUE #1737: EntryMemTable.newEntry: always make a copy
    
    Retaining a reference to that array assumes that the caller
    won't reuse the array for something else -- an assumption
    violated by Journal.scanJournal and probably other callers.
    
    (bug W-5499346)
    (rev cguttapalem)
    Signed-off-by: Samuel Just <sjustsalesforce.com>
    
    Author:
    
    Reviewers: Enrico Olivelli <eolivelligmail.com>, Matteo Merli <mmerliapache.org>, Sijie Guo <sijieapache.org>
    
    This closes #1744 from athanatos/forupstream/wip-1737, closes #1737
    
    (cherry picked from commit df6595804cb1d78f5b531ae2db3a7c751439eb1c)
    
    Author: Sijie Guo <si...@apache.org>
    Author: Enrico Olivelli <eo...@apache.org>
    Author: Matteo Merli <mm...@apache.org>
    Author: Samuel Just <sj...@salesforce.com>
    Author: Nicolas Michael <nm...@salesforce.com>
    Author: Jia Zhai <zh...@apache.org>
    Author: Sijie Guo <gu...@gmail.com>
    Author: cguttapalem <cg...@salesforce.com>
    Author: Ivan Kelly <iv...@apache.org>
    Author: JV Jujjuri <vj...@salesforce.com>
    Author: Andrey Yegorov <dl...@users.noreply.github.com>
    Author: Ivan Kelly <iv...@ivankelly.net>
    Author: Ethan Li <et...@gmail.com>
    Author: arunlakshman <ar...@gmail.com>
    Author: Qi Wang <co...@gmail.com>
    Author: Andrey Yegorov <ay...@salesforce.com>
    
    Reviewers: Enrico Olivelli <eo...@gmail.com>, Sijie Guo <si...@apache.org>
    
    This closes #1746 from athanatos/forupstream/wip-1744-4.7, closes #1737
---
 .../apache/bookkeeper/bookie/EntryMemTable.java    |  9 ++----
 .../bookkeeper/conf/ServerConfiguration.java       | 12 ++++++++
 .../bookkeeper/bookie/BookieJournalTest.java       | 33 ++++++++++++++++++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryMemTable.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryMemTable.java
index 50e93a3..b8653ea 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryMemTable.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryMemTable.java
@@ -352,13 +352,8 @@ public class EntryMemTable {
         int offset = 0;
         int length = entry.remaining();
 
-        if (entry.hasArray()) {
-            buf = entry.array();
-            offset = entry.arrayOffset();
-        } else {
-            buf = new byte[length];
-            entry.get(buf);
-        }
+        buf = new byte[length];
+        entry.get(buf);
         return new EntryKeyValue(ledgerId, entryId, buf, offset, length);
     }
 
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 3e4749c..2588968 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -1544,6 +1544,18 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
     }
 
     /**
+     * Set the max size we should allocate from the skiplist arena. Allocations
+     * larger than this should be allocated directly by the VM to avoid fragmentation.
+     *
+     * @param size max alloc size.
+     * @return server configuration object.
+     */
+    public ServerConfiguration setSkipListArenaMaxAllocSize(int size) {
+        setProperty(SKIP_LIST_MAX_ALLOC_ENTRY, size);
+        return this;
+    }
+
+    /**
      * Should the data be fsynced on journal before acknowledgment.
      *
      * <p>Default is true
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
index 36940ba..5d9de18 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
@@ -641,6 +641,39 @@ public class BookieJournalTest {
     }
 
     /**
+     * Test journal replay with SortedLedgerStorage and a very small max
+     * arena size.
+     */
+    @Test
+    public void testSortedLedgerStorageReplayWithSmallMaxArenaSize() throws Exception {
+        File journalDir = createTempDir("bookie", "journal");
+        Bookie.checkDirectoryStructure(Bookie.getCurrentDirectory(journalDir));
+
+        File ledgerDir = createTempDir("bookie", "ledger");
+        Bookie.checkDirectoryStructure(Bookie.getCurrentDirectory(ledgerDir));
+
+        JournalChannel jc = writeV2Journal(
+                Bookie.getCurrentDirectory(journalDir), 100);
+
+        jc.fc.force(false);
+
+        writeIndexFileForLedger(Bookie.getCurrentDirectory(ledgerDir),
+                1, "testPasswd".getBytes());
+
+        ServerConfiguration conf = TestBKConfiguration.newServerConfiguration();
+        conf.setLedgerStorageClass("org.apache.bookkeeper.bookie.SortedLedgerStorage");
+        conf.setSkipListArenaMaxAllocSize(0);
+        conf.setJournalDirName(journalDir.getPath())
+                .setLedgerDirNames(new String[] { ledgerDir.getPath() });
+
+        Bookie b = new Bookie(conf);
+        b.readJournal();
+        b.ledgerStorage.flush();
+        b.readEntry(1, 80);
+        b.readEntry(1, 99);
+    }
+
+    /**
      * Test partial index (truncate master key) with pre-v3 journals.
      */
     @Test