Repository: bookkeeper
Updated Branches:
  refs/heads/branch-4.3 9cbbd659c -> 49395cf59


BOOKKEEPER-833: EntryLogId and EntryLogLimit should not be larger than 
Integer.MAX_VALUE (sijie)


Project: http://git-wip-us.apache.org/repos/asf/bookkeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/bookkeeper/commit/49395cf5
Tree: http://git-wip-us.apache.org/repos/asf/bookkeeper/tree/49395cf5
Diff: http://git-wip-us.apache.org/repos/asf/bookkeeper/diff/49395cf5

Branch: refs/heads/branch-4.3
Commit: 49395cf5919c03af0f6951055ae1bfd7aef09d04
Parents: 9cbbd65
Author: Sijie Guo <si...@apache.org>
Authored: Tue Apr 21 01:08:20 2015 -0700
Committer: Sijie Guo <si...@apache.org>
Committed: Tue Apr 21 01:08:20 2015 -0700

----------------------------------------------------------------------
 CHANGES.txt                                     |  4 +++
 .../apache/bookkeeper/bookie/EntryLogger.java   | 34 +++++++++++++-------
 .../bookkeeper/bookie/SortedLedgerStorage.java  | 11 ++++++-
 .../bookkeeper/conf/ServerConfiguration.java    |  5 +++
 .../bookkeeper/util/BookKeeperConstants.java    |  7 ++++
 5 files changed, 48 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/49395cf5/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index d3b1e88..ecaf17c 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -44,6 +44,10 @@ Release 4.3.1 - unreleased
 
         BOOKKEEPER-810: Allow to configure TCP connect timeout (Charles Xie 
via sijie)
 
+      bookkeeper-server:
+
+        BOOKKEEPER-833: EntryLogId and EntryLogLimit should not be larger than 
Integer.MAX_VALUE (sijie)
+
 Release 4.3.0 - 2014-10-03
 
   Non-backward compatible changes:

http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/49395cf5/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
----------------------------------------------------------------------
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
index 0e052b5..91ac8b4 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
@@ -22,6 +22,7 @@
 package org.apache.bookkeeper.bookie;
 
 import static com.google.common.base.Charsets.UTF_8;
+import static 
org.apache.bookkeeper.util.BookKeeperConstants.MAX_LOG_SIZE_LIMIT;
 
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
@@ -167,7 +168,7 @@ public class EntryLogger {
             addListener(listener);
         }
         // log size limit
-        this.logSizeLimit = conf.getEntryLogSizeLimit();
+        this.logSizeLimit = Math.min(conf.getEntryLogSizeLimit(), 
MAX_LOG_SIZE_LIMIT);
         this.entryLogPreAllocationEnabled = 
conf.isEntryLogFilePreAllocationEnabled();
 
         // Initialize the entry log header buffer. This cannot be a static 
object
@@ -439,7 +440,12 @@ public class EntryLogger {
             // It would better not to overwrite existing entry log files
             File newLogFile = null;
             do {
-                String logFileName = Long.toHexString(++preallocatedLogId) + 
".log";
+                if (preallocatedLogId >= Integer.MAX_VALUE) {
+                    preallocatedLogId = 0;
+                } else {
+                    ++preallocatedLogId;
+                }
+                String logFileName = Long.toHexString(preallocatedLogId) + 
".log";
                 for (File dir : list) {
                     newLogFile = new File(dir, logFileName);
                     currentDir = dir;
@@ -622,15 +628,16 @@ public class EntryLogger {
     }
 
     synchronized long addEntry(long ledger, ByteBuffer entry, boolean rollLog) 
throws IOException {
-        if (rollLog) {
-            // Create new log if logSizeLimit reached or current disk is full
-            boolean createNewLog = shouldCreateNewEntryLog.get();
-            if (createNewLog || reachEntryLogLimit(entry.remaining() + 4)) {
-                createNewLog();
-                // Reset the flag
-                if (createNewLog) {
-                    shouldCreateNewEntryLog.set(false);
-                }
+        int entrySize = entry.remaining() + 4;
+        boolean reachEntryLogLimit =
+                rollLog ? reachEntryLogLimit(entrySize) : 
readEntryLogHardLimit(entrySize);
+        // Create new log if logSizeLimit reached or current disk is full
+        boolean createNewLog = shouldCreateNewEntryLog.get();
+        if (createNewLog || reachEntryLogLimit) {
+            createNewLog();
+            // Reset the flag
+            if (createNewLog) {
+                shouldCreateNewEntryLog.set(false);
             }
         }
         ByteBuffer buff = ByteBuffer.allocate(4);
@@ -651,6 +658,10 @@ public class EntryLogger {
         return logChannel.position() + size > logSizeLimit;
     }
 
+    synchronized boolean readEntryLogHardLimit(long size) {
+        return logChannel.position() + size > Integer.MAX_VALUE;
+    }
+
     byte[] readEntry(long ledgerId, long entryId, long location) throws 
IOException, Bookie.NoEntryException {
         long entryLogId = logIdForOffset(location);
         long pos = location & 0xffffffffL;
@@ -836,7 +847,6 @@ public class EntryLogger {
             logid2FileChannel.clear();
             // close current writing log file
             closeFileChannel(logChannel);
-            logChannel = null;
         } catch (IOException ie) {
             // we have no idea how to avoid io exception during shutting down, 
so just ignore it
             LOG.error("Error flush entry log during shutting down, which may 
cause entry log corrupted.", ie);

http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/49395cf5/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
----------------------------------------------------------------------
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
index 0a3884c..3e0ce5e 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
@@ -165,13 +165,22 @@ public class SortedLedgerStorage extends 
InterleavedLedgerStorage
         // so we roll entry log files in SortedLedgerStorage itself.
         // After that, we could make the process writing data to entry logger 
file not bound with checkpoint.
         // otherwise, it hurts add performance.
+        //
+        // The only exception for the size limitation is if a file grows to be 
more than hard limit 2GB,
+        // we have to force rolling log, which it might cause slight 
performance effects
         scheduler.submit(new Runnable() {
             @Override
             public void run() {
                 try {
                     LOG.info("Started flushing mem table.");
+                    long logIdBeforeFlush = entryLogger.getCurrentLogId();
                     memTable.flush(SortedLedgerStorage.this);
-                    if (entryLogger.reachEntryLogLimit(0)) {
+                    long logIdAfterFlush = entryLogger.getCurrentLogId();
+                    // in any case that an entry log reaches the limit, we 
roll the log and start checkpointing.
+                    // if a memory table is flushed spanning over two entry 
log files, we also roll log. this is
+                    // for performance consideration: since we don't wanna 
checkpoint a new log file that ledger
+                    // storage is writing to.
+                    if (entryLogger.reachEntryLogLimit(0) || logIdAfterFlush 
!= logIdBeforeFlush) {
                         entryLogger.rollLog();
                         LOG.info("Rolling entry logger since it reached size 
limitation");
                     }

http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/49395cf5/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
----------------------------------------------------------------------
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 03b3be4..f3b1424 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
@@ -23,6 +23,7 @@ import java.util.List;
 import com.google.common.annotations.Beta;
 import org.apache.bookkeeper.stats.NullStatsProvider;
 import org.apache.bookkeeper.stats.StatsProvider;
+import org.apache.bookkeeper.util.BookKeeperConstants;
 import org.apache.bookkeeper.util.ReflectionUtils;
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.lang.StringUtils;
@@ -1356,6 +1357,10 @@ public class ServerConfiguration extends 
AbstractConfiguration {
         if (getJournalAlignmentSize() > getJournalPreAllocSizeMB() * 1024 * 
1024) {
             throw new ConfigurationException("Invalid preallocation size : " + 
getJournalPreAllocSizeMB() + " MB");
         }
+        if (getEntryLogSizeLimit() > BookKeeperConstants.MAX_LOG_SIZE_LIMIT) {
+            throw new ConfigurationException("Entry log file size should not 
be larger than "
+                    + BookKeeperConstants.MAX_LOG_SIZE_LIMIT);
+        }
     }
 
 }

http://git-wip-us.apache.org/repos/asf/bookkeeper/blob/49395cf5/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/BookKeeperConstants.java
----------------------------------------------------------------------
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/BookKeeperConstants.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/BookKeeperConstants.java
index cc86f07..2c2ba38 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/BookKeeperConstants.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/BookKeeperConstants.java
@@ -42,4 +42,11 @@ public class BookKeeperConstants {
     public static final String DEFAULT_ZK_LEDGERS_ROOT_PATH = "/ledgers";
     public static final String LAYOUT_ZNODE = "LAYOUT";
     public static final String INSTANCEID = "INSTANCEID";
+
+    /**
+     * Set the max log size limit to 1GB. It makes extra room for entry log 
file before
+     * hitting hard limit '2GB'. So we don't need to force roll entry log file 
when flushing
+     * memtable (for performance consideration)
+     */
+    public static final long MAX_LOG_SIZE_LIMIT = 1 * 1024 * 1024 * 1024;
 }

Reply via email to