You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by sj...@apache.org on 2018/10/08 17:35:30 UTC

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

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

sjust 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 df65958  ISSUE #1737: EntryMemTable.newEntry: always make a copy
df65958 is described below

commit df6595804cb1d78f5b531ae2db3a7c751439eb1c
Author: Samuel Just <sj...@salesforce.com>
AuthorDate: Mon Oct 8 10:35:25 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 <eo...@gmail.com>, Matteo Merli <mm...@apache.org>, Sijie Guo <si...@apache.org>
    
    This closes #1744 from athanatos/forupstream/wip-1737, 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 7328398..0a95fe9 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
@@ -366,13 +366,8 @@ public class EntryMemTable implements AutoCloseable{
         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 2475084..e9c551e 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
@@ -1756,6 +1756,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 1a0342b..080ebfe 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