[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180642016
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -92,13 +99,10 @@
 private final long logId;
 private final EntryLogMetadata entryLogMetadata;
 private final File logFile;
+private Long ledgerIdAssigned = UNASSIGNED_LEDGERID;
 
 Review comment:
   changed it


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie closed pull request #1328: Create CI jobs for branch-4.7

[GitBox]

sijie closed pull request #1328: Create CI jobs for branch-4.7
URL: https://github.com/apache/bookkeeper/pull/1328
 
 
   

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/.test-infra/jenkins/job_bookkeeper_release_branch.groovy 
b/.test-infra/jenkins/job_bookkeeper_release_branch_46.groovy
similarity index 85%
rename from .test-infra/jenkins/job_bookkeeper_release_branch.groovy
rename to .test-infra/jenkins/job_bookkeeper_release_branch_46.groovy
index 3336a689fb..a632e1517a 100644
--- a/.test-infra/jenkins/job_bookkeeper_release_branch.groovy
+++ b/.test-infra/jenkins/job_bookkeeper_release_branch_46.groovy
@@ -18,9 +18,9 @@
 
 import common_job_properties
 
-// This job runs nightly build for bookkeeper release branch
-mavenJob('bookkeeper_release_branch') {
-  description('Run nightly build for bookkeeper release branch')
+// This job runs nightly build for bookkeeper release branch-4.6
+mavenJob('bookkeeper_release_branch_46') {
+  description('Run nightly build for bookkeeper release branch-4.6')
 
   // Set common parameters.
   common_job_properties.setTopLevelMainJobProperties(
@@ -36,8 +36,8 @@ mavenJob('bookkeeper_release_branch') {
   // Allows triggering this build against pull requests.
   common_job_properties.enablePhraseTriggeringFromPullRequest(
   delegate,
-  'Release Branch Test',
-  '/test-release-branch')
+  'Release Branch 4.6 Test',
+  '/test-release-branch-46')
   
   // Set maven parameters.
   common_job_properties.setMavenConfig(delegate)
diff --git a/.test-infra/jenkins/job_bookkeeper_release_branch_47_java8.groovy 
b/.test-infra/jenkins/job_bookkeeper_release_branch_47_java8.groovy
new file mode 100644
index 00..5a941c09b8
--- /dev/null
+++ b/.test-infra/jenkins/job_bookkeeper_release_branch_47_java8.groovy
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import common_job_properties
+
+// This job runs the Java postcommit tests on Java 8 for branch-4.7
+mavenJob('bookkeeper_release_branch_47_java8') {
+  description('Runs nightly build for bookkeeper branch-4.7 in Java 8.')
+
+  // Set common parameters.
+  common_job_properties.setTopLevelMainJobProperties(
+delegate, 'branch-4.7', 'JDK 1.8 (latest)')
+
+  // Sets that this is a PostCommit job.
+  common_job_properties.setPostCommit(
+  delegate,
+  'H 12 * * *',
+  false)
+
+  // Allows triggering this build against pull requests.
+  common_job_properties.enablePhraseTriggeringFromPullRequest(
+  delegate,
+  'Release Branch 4.7 Java 8 Test',
+  '/test-release-branch-47-java8')
+
+  // Set maven parameters.
+  common_job_properties.setMavenConfig(delegate)
+
+  // Maven build project.
+  goals('clean apache-rat:check package spotbugs:check -Ddistributedlog 
-Dstream -DstreamTests')
+}
diff --git a/.test-infra/jenkins/job_bookkeeper_release_branch_47_java9.groovy 
b/.test-infra/jenkins/job_bookkeeper_release_branch_47_java9.groovy
new file mode 100644
index 00..968e6cdb20
--- /dev/null
+++ b/.test-infra/jenkins/job_bookkeeper_release_branch_47_java9.groovy
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import common_job_properties
+
+// This job runs the Java postcommit 

[GitHub] sijie commented on issue #1328: Create CI jobs for branch-4.7

[GitBox]

sijie commented on issue #1328: Create CI jobs for branch-4.7
URL: https://github.com/apache/bookkeeper/pull/1328#issuecomment-380334666
 
 
   IGNORE CI


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie opened a new pull request #1329: (WIP) Issue 1246: Scripting the whole release procedure

[GitBox]

sijie opened a new pull request #1329: (WIP) Issue 1246: Scripting the whole 
release procedure
URL: https://github.com/apache/bookkeeper/pull/1329
 
 
   Descriptions of the changes in this PR:
   
   *Motivation*
   
   Releasing bookkeeper becomes tricky after introducing `circe-checksum`, 
since we need to ensure linux jni library is shipped as part of 
`circe-checksum`.
   
   *Solution*
   
   - switch to use docker for generating bookkeeper release
   - scripting the whole release procedure as many as possible
   
   Master Issue: #1246 
   
   > ---
   > Be sure to do all of the following to help us incorporate your contribution
   > quickly and easily:
   >
   > If this PR is a BookKeeper Proposal (BP):
   >
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of bookkeeper proposal`
   > `e.g. BP-1: 64 bits ledger is support`
   > - [ ] Attach the master issue link in the description of this PR.
   > - [ ] Attach the google doc link if the BP is written in Google Doc.
   >
   > Otherwise:
   > 
   > - [ ] Make sure the PR title is formatted like:
   > `: Description of pull request`
   > `e.g. Issue 123: Description ...`
   > - [ ] Make sure tests pass via `mvn clean apache-rat:check install 
spotbugs:check`.
   > - [ ] Replace `` in the title with the actual Issue number.
   > 
   > ---
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180593414
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
+: 
ledgerDirsManager.isDirFull(logChannel.getLogFile().getParentFile());
+boolean allDisksFull = !ledgerDirsManager.hasWritableLedgerDirs();
+
+  

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180590406
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
+: 
ledgerDirsManager.isDirFull(logChannel.getLogFile().getParentFile());
+boolean allDisksFull = !ledgerDirsManager.hasWritableLedgerDirs();
+
+  

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180588670
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
+: 
ledgerDirsManager.isDirFull(logChannel.getLogFile().getParentFile());
+boolean allDisksFull = !ledgerDirsManager.hasWritableLedgerDirs();
+
+  

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180584833
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
 
 Review comment:
   will do


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180583759
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -120,21 +124,104 @@ public ConcurrentLongLongHashMap getLedgersMap() {
 return entryLogMetadata.getLedgersMap();
 }
 
+public Long getLedgerIdAssigned() {
+return ledgerIdAssigned;
+}
+
+public void setLedgerIdAssigned(Long ledgerId) {
+this.ledgerIdAssigned = ledgerId;
+}
+
 @Override
 public String toString() {
 return MoreObjects.toStringHelper(BufferedChannel.class)
 .add("logId", logId)
 .add("logFile", logFile)
+.add("ledgerIdAssigned", ledgerIdAssigned)
 .toString();
 }
+
+/**
+ * Append the ledger map at the end of the entry log.
+ * Updates the entry log file header with the offset and size of the 
map.
+ */
+private void appendLedgersMap() throws IOException {
+
+long ledgerMapOffset = this.position();
+
+ConcurrentLongLongHashMap ledgersMap = this.getLedgersMap();
+int numberOfLedgers = (int) ledgersMap.size();
+
+// Write the ledgers map into several batches
+
+final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
+final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
+
+try {
+ledgersMap.forEach(new BiConsumerLong() {
+int remainingLedgers = numberOfLedgers;
+boolean startNewBatch = true;
+int remainingInBatch = 0;
+
+@Override
+public void accept(long ledgerId, long size) {
+if (startNewBatch) {
+int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
+int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
+
+serializedMap.clear();
+serializedMap.writeInt(ledgerMapSize - 4);
+serializedMap.writeLong(INVALID_LID);
+serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
+serializedMap.writeInt(batchSize);
+
+startNewBatch = false;
+remainingInBatch = batchSize;
+}
+// Dump the ledger in the current batch
+serializedMap.writeLong(ledgerId);
+serializedMap.writeLong(size);
+--remainingLedgers;
+
+if (--remainingInBatch == 0) {
+// Close current batch
+try {
+write(serializedMap);
+} catch (IOException e) {
+throw new RuntimeException(e);
+}
+
+startNewBatch = true;
+}
+}
+});
+} catch (RuntimeException e) {
+if (e.getCause() instanceof IOException) {
+throw (IOException) e.getCause();
+} else {
+throw e;
+}
+} finally {
+serializedMap.release();
+}
+// Flush the ledger's map out before we write the header.
+// Otherwise the header might point to something that is not fully
+// written
+super.flush();
+
+// Update the headers with the map offset and count of ledgers
+ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
+mapInfo.putLong(ledgerMapOffset);
+mapInfo.putInt(numberOfLedgers);
+mapInfo.flip();
+this.fileChannel.write(mapInfo, LEDGERS_MAP_OFFSET_POSITION);
+}
 }
 
-volatile File currentDir;
 private final LedgerDirsManager ledgerDirsManager;
 private final boolean entryLogPerLedgerEnabled;
-private final AtomicBoolean shouldCreateNewEntryLog = new 
AtomicBoolean(false);
 
-private volatile long leastUnflushedLogId;
+RecentEntryLogsStatus recentlyCreatedEntryLogsStatus;
 
 Review comment:
   done


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:
us...@infra.apache.org


With regards,

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180583624
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -84,6 +88,9 @@
  */
 public class EntryLogger {
 private static final Logger LOG = 
LoggerFactory.getLogger(EntryLogger.class);
+static final Long UNASSIGNED_LEDGERID = Long.valueOf(-1);
 
 Review comment:
   changed it to long


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180581487
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyEntryLogger.java
 ##
 @@ -54,7 +49,7 @@ protected boolean removeEntryLog(long entryLogId) {
 }
 
 @Override
-public synchronized long addEntry(long ledger, ByteBuffer entry) throws 
IOException {
+public synchronized long addEntry(Long ledgerId, ByteBuffer entry) throws 
IOException {
 
 Review comment:
   ok..will revert it back to long primitive datatype in interface.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180565394
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CreateNewLogTest.java
 ##
 @@ -93,16 +98,16 @@ public void testCreateNewLog() throws Exception {
 // Extracted from createNewLog()
 String logFileName = Long.toHexString(1) + ".log";
 File dir = ledgerDirsManager.pickRandomWritableDir();
-LOG.info("Picked this directory: " + dir);
+LOG.info("Picked this directory: {}", dir);
 File newLogFile = new File(dir, logFileName);
 newLogFile.createNewFile();
 
 EntryLogger el = new EntryLogger(conf, ledgerDirsManager);
 // Calls createNewLog, and with the number of directories we
 // are using, if it picks one at random it will fail.
-el.createNewLog();
-LOG.info("This is the current log id: " + el.getCurrentLogId());
-assertTrue("Wrong log id", el.getCurrentLogId() > 1);
+((EntryLogManagerBase) el.entryLogManager).createNewLog(0L);
 
 Review comment:
   1) I can add simple getter for EntryLogManager in EntryLogger, but wondering 
how would it help us in mocking if inside EntryLogger class direct reference of 
EntryLogger instance is used and using getter of EntryLogManager in all the 
places it is used in EntryLogger class is a stretch. 
   2) if mocking is the requirement here then i can create 
initializeEntryLogManager method, which would initialize the EntryLogManager 
depending on the config, and for mocking purpose that method can be mocked to 
return what we want.
   3) ok. in some places I wanted to test  getPreviousAllocatedEntryLogId, but 
it seems in this test it is not appropriate to change it to 
getPreviousAllocatedEntryLogId. will revert it back.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie opened a new pull request #1328: Create CI jobs for branch-4.7

[GitBox]

sijie opened a new pull request #1328: Create CI jobs for branch-4.7
URL: https://github.com/apache/bookkeeper/pull/1328
 
 
   Descriptions of the changes in this PR:
   
   We need to CI jobs running for subsequent minor releases on branch-4.7.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180555218
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##
 @@ -173,7 +175,7 @@ public void checkpoint(final Checkpoint checkpoint) throws 
IOException {
 // it means bytes might live at current active entry log, we need
 // roll current entry log and then issue checkpoint to underlying
 // interleaved ledger storage.
-entryLogger.rollLog();
+entryLogger.rollLogs();
 
 Review comment:
   @sijie i'm flexible here to make the change but
   
   1) to begin with, from what i understand, this check (numBytesFlushed > 0) 
and rollLogs call is not needed. Since entries which come before checkpoint are 
already flushed from memtable (because of the way 
sortedledgerstorage.checkpoint) is called.
   2) im not super convinced with moving from rollLogs method in interface to 
prepareCheckpoint, because this has meaning/use only for sortedledgerstorage 
and for single entrylog manager and having such a generic method name 
"prepareCheckpoint" seems to be stretch.
   3) in the method signature - "void prepareCheckpoint(Checkpoint checkpoint, 
long numBytesFlushedBetweenCheckpoint)" 'checkpoint' is not needed and we can 
remove it. parameter name 'numBytesFlushedBetweenCheckpoint' is not appropriate 
since it is just bytes added between previous flush and this checkpoint.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180555218
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##
 @@ -173,7 +175,7 @@ public void checkpoint(final Checkpoint checkpoint) throws 
IOException {
 // it means bytes might live at current active entry log, we need
 // roll current entry log and then issue checkpoint to underlying
 // interleaved ledger storage.
-entryLogger.rollLog();
+entryLogger.rollLogs();
 
 Review comment:
   @sijie i'm flexible here to make the change but
   
   1) to begin with, from what i understand, this check (numBytesFlushed > 0) 
and rollLogs call is not needed. Since entries which come before checkpoint are 
already flushed from memtable (because of the way 
sortedledgerstorage.checkpoint) is called.
   2) im not super convinced with moving from rollLogs method in interface to 
prepareCheckpoint, because this has meaning/use only for sortedledgerstorage 
and for single entrylog manager and having such a generic method name 
"prepareCheckpoint" seems to be a stretch.
   3) in the method signature - "void prepareCheckpoint(Checkpoint checkpoint, 
long numBytesFlushedBetweenCheckpoint)" 'checkpoint' is not needed and we can 
remove it. parameter name 'numBytesFlushedBetweenCheckpoint' is not appropriate 
since it is just bytes added between previous flush and this checkpoint.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on a change in pull request #1281: Issue #570: 
Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180548567
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##
 @@ -78,6 +79,7 @@ public void initialize(ServerConfiguration conf,
 .setNameFormat("SortedLedgerStorage-%d")
 .setPriority((Thread.NORM_PRIORITY + Thread.MAX_PRIORITY) / 
2).build());
 this.stateManager = stateManager;
+this.isTransactionalCompactionEnabled = 
conf.getUseTransactionalCompaction();
 
 Review comment:
   yes, it is not needed. Will remove.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on issue #914: Document bookkeeper shell command changes

[GitBox]

sijie commented on issue #914: Document bookkeeper shell command changes
URL: https://github.com/apache/bookkeeper/issues/914#issuecomment-380217833
 
 
   This is done by #1325 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie closed issue #914: Document bookkeeper shell command changes

[GitBox]

sijie closed issue #914: Document bookkeeper shell command changes
URL: https://github.com/apache/bookkeeper/issues/914
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180524406
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##
 @@ -173,7 +175,7 @@ public void checkpoint(final Checkpoint checkpoint) throws 
IOException {
 // it means bytes might live at current active entry log, we need
 // roll current entry log and then issue checkpoint to underlying
 // interleaved ledger storage.
-entryLogger.rollLog();
+entryLogger.rollLogs();
 
 Review comment:
   I think `numBytesFlushed > 0` logic is a behavior that specific to 
single-log manager. `rollLogs` is a not proper term for the meaning here. I 
would suggest adding a method in the EntryLogManager:
   
   `void prepareCheckpoint(Checkpoint checkpoint, long 
numBytesFlushedBetweenCheckpoints)` -> it means prepare checkpoint in the entry 
logger. 
   
   Then logic here can be changed to:
   
   ```
   long numBytesFlushed = memTable.flush(this, checkpoint);
   entryLogger.prepareCheckpoint(checkpoint, numBytesFlushed);
   super.checkpoint(checkpoint);
   ```
   
   in single-log manager, you can implement 
   
   ```
   void prepareCheckpoint(Checkpoint checkpoint, long 
numBytesFlushedBetweenCheckpoint) {
   if (numBytesFlushedBetweenCheckpoint > 0) {
   rollLogs();
   }
   }
   ```
   
   in per-ledger-log manager, it is a no-op.
   
   so in this way, you can get rid of `rollLogs` in EntryLogManager.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180514001
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
 
 Review comment:
   I am not sure we need to put this rotatedLogChannels in this base class. for 
two reasons, 1) there is really no commonality between single-log manager and 
per-ledger-log manager around checkpoint/rotation.  I don't think it is 
necessarily to put them in the single model. 2) I would prefer using an ordered 
structure (before this change, it is a `list`), because entry log file rotation 
in single-log manager is in order and checkpoint logic is in order. changing 
single-log manager to use `Set` could expose unknown potential bugs, which is a 
risky change to me.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180514235
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
+: 
ledgerDirsManager.isDirFull(logChannel.getLogFile().getParentFile());
+boolean allDisksFull = !ledgerDirsManager.hasWritableLedgerDirs();
+
+

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180515728
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
+: 
ledgerDirsManager.isDirFull(logChannel.getLogFile().getParentFile());
+boolean allDisksFull = !ledgerDirsManager.hasWritableLedgerDirs();
+
+

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180516776
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
 
 Review comment:
   `flushRotatedLogs` and `flushCurrentLogs` are only used in `flush()` method, 
why not just provide `flush()` method in EntryLogManager, like `checkpoint`.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180505515
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -120,21 +124,104 @@ public ConcurrentLongLongHashMap getLedgersMap() {
 return entryLogMetadata.getLedgersMap();
 }
 
+public Long getLedgerIdAssigned() {
+return ledgerIdAssigned;
+}
+
+public void setLedgerIdAssigned(Long ledgerId) {
+this.ledgerIdAssigned = ledgerId;
+}
+
 @Override
 public String toString() {
 return MoreObjects.toStringHelper(BufferedChannel.class)
 .add("logId", logId)
 .add("logFile", logFile)
+.add("ledgerIdAssigned", ledgerIdAssigned)
 .toString();
 }
+
+/**
+ * Append the ledger map at the end of the entry log.
+ * Updates the entry log file header with the offset and size of the 
map.
+ */
+private void appendLedgersMap() throws IOException {
+
+long ledgerMapOffset = this.position();
+
+ConcurrentLongLongHashMap ledgersMap = this.getLedgersMap();
+int numberOfLedgers = (int) ledgersMap.size();
+
+// Write the ledgers map into several batches
+
+final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
+final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
+
+try {
+ledgersMap.forEach(new BiConsumerLong() {
+int remainingLedgers = numberOfLedgers;
+boolean startNewBatch = true;
+int remainingInBatch = 0;
+
+@Override
+public void accept(long ledgerId, long size) {
+if (startNewBatch) {
+int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
+int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
+
+serializedMap.clear();
+serializedMap.writeInt(ledgerMapSize - 4);
+serializedMap.writeLong(INVALID_LID);
+serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
+serializedMap.writeInt(batchSize);
+
+startNewBatch = false;
+remainingInBatch = batchSize;
+}
+// Dump the ledger in the current batch
+serializedMap.writeLong(ledgerId);
+serializedMap.writeLong(size);
+--remainingLedgers;
+
+if (--remainingInBatch == 0) {
+// Close current batch
+try {
+write(serializedMap);
+} catch (IOException e) {
+throw new RuntimeException(e);
+}
+
+startNewBatch = true;
+}
+}
+});
+} catch (RuntimeException e) {
+if (e.getCause() instanceof IOException) {
+throw (IOException) e.getCause();
+} else {
+throw e;
+}
+} finally {
+serializedMap.release();
+}
+// Flush the ledger's map out before we write the header.
+// Otherwise the header might point to something that is not fully
+// written
+super.flush();
+
+// Update the headers with the map offset and count of ledgers
+ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
+mapInfo.putLong(ledgerMapOffset);
+mapInfo.putInt(numberOfLedgers);
+mapInfo.flip();
+this.fileChannel.write(mapInfo, LEDGERS_MAP_OFFSET_POSITION);
+}
 }
 
-volatile File currentDir;
 private final LedgerDirsManager ledgerDirsManager;
 private final boolean entryLogPerLedgerEnabled;
-private final AtomicBoolean shouldCreateNewEntryLog = new 
AtomicBoolean(false);
 
-private volatile long leastUnflushedLogId;
+RecentEntryLogsStatus recentlyCreatedEntryLogsStatus;
 
 Review comment:
   final


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:
us...@infra.apache.org


With regards,
Apache 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180520311
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java
 ##
 @@ -78,6 +79,7 @@ public void initialize(ServerConfiguration conf,
 .setNameFormat("SortedLedgerStorage-%d")
 .setPriority((Thread.NORM_PRIORITY + Thread.MAX_PRIORITY) / 
2).build());
 this.stateManager = stateManager;
+this.isTransactionalCompactionEnabled = 
conf.getUseTransactionalCompaction();
 
 Review comment:
   I don't think we need this flag any more, no?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180520230
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ReadOnlyEntryLogger.java
 ##
 @@ -54,7 +49,7 @@ protected boolean removeEntryLog(long entryLogId) {
 }
 
 @Override
-public synchronized long addEntry(long ledger, ByteBuffer entry) throws 
IOException {
+public synchronized long addEntry(Long ledgerId, ByteBuffer entry) throws 
IOException {
 
 Review comment:
   @reddycharan you can still use long for retreiving from a Long hashmap. why 
do you need to change here? in any cases long to Long is a boxing operation, if 
single-log manager doesn't need Long, it should be deferred to where it is 
really needed.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180517926
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
 
 Review comment:
   the `closeCurrentLogs` and `forceCloseCurrentLogs` are the abstraction of 
the logic during `closing`. I think it is better to call it `close` and 
`closeNow` (or `forceClose`) - meaning closing the manager, force closing the 
manager. This would provide a better meaning and it is similar as `shutdown` 
and `shutdownNow` in scheduler.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180525552
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CreateNewLogTest.java
 ##
 @@ -93,16 +98,16 @@ public void testCreateNewLog() throws Exception {
 // Extracted from createNewLog()
 String logFileName = Long.toHexString(1) + ".log";
 File dir = ledgerDirsManager.pickRandomWritableDir();
-LOG.info("Picked this directory: " + dir);
+LOG.info("Picked this directory: {}", dir);
 File newLogFile = new File(dir, logFileName);
 newLogFile.createNewFile();
 
 EntryLogger el = new EntryLogger(conf, ledgerDirsManager);
 // Calls createNewLog, and with the number of directories we
 // are using, if it picks one at random it will fail.
-el.createNewLog();
-LOG.info("This is the current log id: " + el.getCurrentLogId());
-assertTrue("Wrong log id", el.getCurrentLogId() > 1);
+((EntryLogManagerBase) el.entryLogManager).createNewLog(0L);
 
 Review comment:
   - nit: provide a `getEntryLogManager` in EntryLogger. so we can mock it in 
future if needed.
   - better to cast to `SingleEntryLogManager`. because you can expose 
`getCurrentLogId` in `SingleEntryLogManager`.
   
   ```
   SingleEntryLogManager selm = (SingleEntryLogManager) 
(el.getEntryLogManager());
   selm.createNewLog(0L);
   assertTrue(selm.getCurrentLogId() > 1);
   ```


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180507966
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -499,176 +576,163 @@ void createNewLog() throws IOException {
 EntryLoggerAllocator getEntryLoggerAllocator() {
 return entryLoggerAllocator;
 }
-/**
- * Append the ledger map at the end of the entry log.
- * Updates the entry log file header with the offset and size of the map.
- */
-private void appendLedgersMap(BufferedLogChannel entryLogChannel) throws 
IOException {
-long ledgerMapOffset = entryLogChannel.position();
-
-ConcurrentLongLongHashMap ledgersMap = entryLogChannel.getLedgersMap();
-int numberOfLedgers = (int) ledgersMap.size();
-
-// Write the ledgers map into several batches
-
-final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
-final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
-
-try {
-ledgersMap.forEach(new BiConsumerLong() {
-int remainingLedgers = numberOfLedgers;
-boolean startNewBatch = true;
-int remainingInBatch = 0;
-
-@Override
-public void accept(long ledgerId, long size) {
-if (startNewBatch) {
-int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
-int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
-
-serializedMap.clear();
-serializedMap.writeInt(ledgerMapSize - 4);
-serializedMap.writeLong(INVALID_LID);
-serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
-serializedMap.writeInt(batchSize);
-
-startNewBatch = false;
-remainingInBatch = batchSize;
-}
-// Dump the ledger in the current batch
-serializedMap.writeLong(ledgerId);
-serializedMap.writeLong(size);
---remainingLedgers;
-
-if (--remainingInBatch == 0) {
-// Close current batch
-try {
-entryLogChannel.write(serializedMap);
-} catch (IOException e) {
-throw new RuntimeException(e);
-}
-
-startNewBatch = true;
-}
-}
-});
-} catch (RuntimeException e) {
-if (e.getCause() instanceof IOException) {
-throw (IOException) e.getCause();
-} else {
-throw e;
-}
-} finally {
-serializedMap.release();
-}
-// Flush the ledger's map out before we write the header.
-// Otherwise the header might point to something that is not fully 
written
-entryLogChannel.flush();
-
-// Update the headers with the map offset and count of ledgers
-ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
-mapInfo.putLong(ledgerMapOffset);
-mapInfo.putInt(numberOfLedgers);
-mapInfo.flip();
-entryLogChannel.fileChannel.write(mapInfo, 
LEDGERS_MAP_OFFSET_POSITION);
-}
 
 /**
  * An allocator pre-allocates entry log files.
  */
 class EntryLoggerAllocator {
 
 private long preallocatedLogId;
-private Future preallocation = null;
+Future preallocation = null;
 private ExecutorService allocatorExecutor;
-private final Object createEntryLogLock = new Object();
-private final Object createCompactionLogLock = new Object();
 
 EntryLoggerAllocator(long logId) {
 preallocatedLogId = logId;
 allocatorExecutor = Executors.newSingleThreadExecutor();
 }
 
-BufferedLogChannel createNewLog() throws IOException {
-synchronized (createEntryLogLock) {
-BufferedLogChannel bc;
-if (!entryLogPreAllocationEnabled){
-// create a new log directly
-bc = allocateNewLog();
-return bc;
-} else {
-// allocate directly to response request
-if (null == preallocation){
-bc = allocateNewLog();
+synchronized long getPreallocatedLogId(){
+return preallocatedLogId;
+}
+
+synchronized BufferedLogChannel createNewLog() throws IOException {
+BufferedLogChannel bc;
+if (!entryLogPreAllocationEnabled || null == 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180510817
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -880,39 +1195,6 @@ protected ByteBuf initialValue() throws Exception {
 }
 };
 
-public synchronized long addEntry(long ledger, ByteBuf entry, boolean 
rollLog) throws IOException {
-if (null == logChannel) {
-// log channel can be null because the file is deferred to be 
created when no writable ledger directory
-// is available.
-createNewLog();
-}
-
-int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
-boolean reachEntryLogLimit =
-rollLog ? reachEntryLogLimit(entrySize) : 
readEntryLogHardLimit(entrySize);
-// Create new log if logSizeLimit reached or current disk is full
-boolean createNewLog = shouldCreateNewEntryLog.get();
 
 Review comment:
   any reason why `shouldCreateNewEntryLog` is removed?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180510213
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
+
+/*
+ * flush rotated logs.
+ */
+void flushRotatedLogs() throws IOException;
+
+/*
+ * flush current logs.
+ */
+void flushCurrentLogs() throws IOException;
+
+/*
+ * close current logs.
+ */
+void closeCurrentLogs() throws IOException;
+
+/*
+ * force close current logs.
+ */
+void forceCloseCurrentLogs();
+
+/*
+ * this method should be called before doing entrymemtable flush, it
+ * would save the state of the entrylogger before entrymemtable flush
+ * and commitEntryMemTableFlush would take appropriate action after
+ * entrymemtable flush.
+ */
+void prepareEntryMemTableFlush();
+
 /*
- * In the case of entryLogPerLedgerEnabled we need to flush both
- * rotatedlogs and currentlogs. This is needed because syncThread
- * periodically does checkpoint and at this time all the logs should
- * be flushed.
+ * this method should be called after doing entrymemtable flush,it 
would
+ * take appropriate action after entrymemtable flush depending on the
+ * current state of the entrylogger and the state of the entrylogger
+ * during prepareEntryMemTableFlush.
  *
- * TODO: When EntryLogManager is introduced in the subsequent 
sub-tasks of
- * this Issue, I will move this logic to individual implamentations of
- * EntryLogManager and it would be free of this booalen flag based 
logic.
+ * It is assumed that there would be corresponding
+ * prepareEntryMemTableFlush for every commitEntryMemTableFlush and 
both
+ * would be called from the same thread.
  *
+ * returns boolean value indicating whether EntryMemTable should do 
checkpoint
+ * after this commit method.
  */
-if (entryLogPerLedgerEnabled) {
-flushCurrentLog();
-}
+boolean commitEntryMemTableFlush() throws IOException;
 }
 
-void flushRotatedLogs() throws IOException {
-List channels = null;
-long flushedLogId = INVALID_LID;
-synchronized (this) {
-channels = logChannelsToFlush;
-logChannelsToFlush = null;
+abstract class EntryLogManagerBase implements EntryLogManager {
+final Set rotatedLogChannels;
+
+EntryLogManagerBase() {
+rotatedLogChannels = ConcurrentHashMap.newKeySet();
 }
-if (null == channels) {
-return;
+
+/*
+ * This method should be guarded by a lock, so callers of this method
+ * should be in the right scope of the lock.
+ */
+@Override
+public long addEntry(Long ledger, ByteBuf entry, boolean rollLog) 
throws IOException {
+
+int entrySize = entry.readableBytes() + 4; // Adding 4 bytes to 
prepend the size
+BufferedLogChannel logChannel = getCurrentLogForLedger(ledger);
+boolean reachEntryLogLimit = rollLog ? 
reachEntryLogLimit(logChannel, entrySize)
+: readEntryLogHardLimit(logChannel, entrySize);
+// Create new log if logSizeLimit reached or current disk is full
+boolean diskFull = (logChannel == null) ? false
 
 Review comment:
   you are changing logic while moving code around for abstracting interface. 
This makes review really hard and painful.  you should just move the code first 
and 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180507116
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -499,176 +576,163 @@ void createNewLog() throws IOException {
 EntryLoggerAllocator getEntryLoggerAllocator() {
 return entryLoggerAllocator;
 }
-/**
- * Append the ledger map at the end of the entry log.
- * Updates the entry log file header with the offset and size of the map.
- */
-private void appendLedgersMap(BufferedLogChannel entryLogChannel) throws 
IOException {
-long ledgerMapOffset = entryLogChannel.position();
-
-ConcurrentLongLongHashMap ledgersMap = entryLogChannel.getLedgersMap();
-int numberOfLedgers = (int) ledgersMap.size();
-
-// Write the ledgers map into several batches
-
-final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
-final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
-
-try {
-ledgersMap.forEach(new BiConsumerLong() {
-int remainingLedgers = numberOfLedgers;
-boolean startNewBatch = true;
-int remainingInBatch = 0;
-
-@Override
-public void accept(long ledgerId, long size) {
-if (startNewBatch) {
-int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
-int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
-
-serializedMap.clear();
-serializedMap.writeInt(ledgerMapSize - 4);
-serializedMap.writeLong(INVALID_LID);
-serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
-serializedMap.writeInt(batchSize);
-
-startNewBatch = false;
-remainingInBatch = batchSize;
-}
-// Dump the ledger in the current batch
-serializedMap.writeLong(ledgerId);
-serializedMap.writeLong(size);
---remainingLedgers;
-
-if (--remainingInBatch == 0) {
-// Close current batch
-try {
-entryLogChannel.write(serializedMap);
-} catch (IOException e) {
-throw new RuntimeException(e);
-}
-
-startNewBatch = true;
-}
-}
-});
-} catch (RuntimeException e) {
-if (e.getCause() instanceof IOException) {
-throw (IOException) e.getCause();
-} else {
-throw e;
-}
-} finally {
-serializedMap.release();
-}
-// Flush the ledger's map out before we write the header.
-// Otherwise the header might point to something that is not fully 
written
-entryLogChannel.flush();
-
-// Update the headers with the map offset and count of ledgers
-ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
-mapInfo.putLong(ledgerMapOffset);
-mapInfo.putInt(numberOfLedgers);
-mapInfo.flip();
-entryLogChannel.fileChannel.write(mapInfo, 
LEDGERS_MAP_OFFSET_POSITION);
-}
 
 /**
  * An allocator pre-allocates entry log files.
  */
 class EntryLoggerAllocator {
 
 private long preallocatedLogId;
-private Future preallocation = null;
+Future preallocation = null;
 private ExecutorService allocatorExecutor;
-private final Object createEntryLogLock = new Object();
-private final Object createCompactionLogLock = new Object();
 
 EntryLoggerAllocator(long logId) {
 preallocatedLogId = logId;
 allocatorExecutor = Executors.newSingleThreadExecutor();
 }
 
-BufferedLogChannel createNewLog() throws IOException {
-synchronized (createEntryLogLock) {
-BufferedLogChannel bc;
-if (!entryLogPreAllocationEnabled){
-// create a new log directly
-bc = allocateNewLog();
-return bc;
-} else {
-// allocate directly to response request
-if (null == preallocation){
-bc = allocateNewLog();
+synchronized long getPreallocatedLogId(){
+return preallocatedLogId;
+}
+
+synchronized BufferedLogChannel createNewLog() throws IOException {
+BufferedLogChannel bc;
+if (!entryLogPreAllocationEnabled || null == 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180516551
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -788,89 +852,340 @@ private long readLastLogId(File f) {
 }
 }
 
-/**
- * Flushes all rotated log channels. After log channels are flushed,
- * move leastUnflushedLogId ptr to current logId.
- */
-void checkpoint() throws IOException {
-flushRotatedLogs();
+interface EntryLogManager {
+
+/*
+ * add entry to the corresponding entrylog and return the position of
+ * the entry in the entrylog
+ */
+long addEntry(Long ledger, ByteBuf entry, boolean rollLog) throws 
IOException;
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * Returns eligible writable ledger dir for the creation next entrylog
+ */
+File getDirForNextEntryLog(List writableLedgerDirs);
+
+/*
+ * Do the operations required for checkpoint.
+ */
+void checkpoint() throws IOException;
+
+/*
+ * roll entryLogs.
+ */
+void rollLogs() throws IOException;
 
 Review comment:
   after the change of `prepareMemtableFlush` and `commitMemtableFlush`, 
rollLogs seems to be only the behavior of single-log manager. I don't think you 
need `rollLogs` here, unless I missed something.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180508758
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -499,176 +576,163 @@ void createNewLog() throws IOException {
 EntryLoggerAllocator getEntryLoggerAllocator() {
 return entryLoggerAllocator;
 }
-/**
- * Append the ledger map at the end of the entry log.
- * Updates the entry log file header with the offset and size of the map.
- */
-private void appendLedgersMap(BufferedLogChannel entryLogChannel) throws 
IOException {
-long ledgerMapOffset = entryLogChannel.position();
-
-ConcurrentLongLongHashMap ledgersMap = entryLogChannel.getLedgersMap();
-int numberOfLedgers = (int) ledgersMap.size();
-
-// Write the ledgers map into several batches
-
-final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
-final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
-
-try {
-ledgersMap.forEach(new BiConsumerLong() {
-int remainingLedgers = numberOfLedgers;
-boolean startNewBatch = true;
-int remainingInBatch = 0;
-
-@Override
-public void accept(long ledgerId, long size) {
-if (startNewBatch) {
-int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
-int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
-
-serializedMap.clear();
-serializedMap.writeInt(ledgerMapSize - 4);
-serializedMap.writeLong(INVALID_LID);
-serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
-serializedMap.writeInt(batchSize);
-
-startNewBatch = false;
-remainingInBatch = batchSize;
-}
-// Dump the ledger in the current batch
-serializedMap.writeLong(ledgerId);
-serializedMap.writeLong(size);
---remainingLedgers;
-
-if (--remainingInBatch == 0) {
-// Close current batch
-try {
-entryLogChannel.write(serializedMap);
-} catch (IOException e) {
-throw new RuntimeException(e);
-}
-
-startNewBatch = true;
-}
-}
-});
-} catch (RuntimeException e) {
-if (e.getCause() instanceof IOException) {
-throw (IOException) e.getCause();
-} else {
-throw e;
-}
-} finally {
-serializedMap.release();
-}
-// Flush the ledger's map out before we write the header.
-// Otherwise the header might point to something that is not fully 
written
-entryLogChannel.flush();
-
-// Update the headers with the map offset and count of ledgers
-ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
-mapInfo.putLong(ledgerMapOffset);
-mapInfo.putInt(numberOfLedgers);
-mapInfo.flip();
-entryLogChannel.fileChannel.write(mapInfo, 
LEDGERS_MAP_OFFSET_POSITION);
-}
 
 /**
  * An allocator pre-allocates entry log files.
  */
 class EntryLoggerAllocator {
 
 private long preallocatedLogId;
-private Future preallocation = null;
+Future preallocation = null;
 private ExecutorService allocatorExecutor;
-private final Object createEntryLogLock = new Object();
-private final Object createCompactionLogLock = new Object();
 
 EntryLoggerAllocator(long logId) {
 preallocatedLogId = logId;
 allocatorExecutor = Executors.newSingleThreadExecutor();
 }
 
-BufferedLogChannel createNewLog() throws IOException {
-synchronized (createEntryLogLock) {
-BufferedLogChannel bc;
-if (!entryLogPreAllocationEnabled){
-// create a new log directly
-bc = allocateNewLog();
-return bc;
-} else {
-// allocate directly to response request
-if (null == preallocation){
-bc = allocateNewLog();
+synchronized long getPreallocatedLogId(){
+return preallocatedLogId;
+}
+
+synchronized BufferedLogChannel createNewLog() throws IOException {
+BufferedLogChannel bc;
+if (!entryLogPreAllocationEnabled || null == 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180508334
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -499,176 +576,163 @@ void createNewLog() throws IOException {
 EntryLoggerAllocator getEntryLoggerAllocator() {
 return entryLoggerAllocator;
 }
-/**
- * Append the ledger map at the end of the entry log.
- * Updates the entry log file header with the offset and size of the map.
- */
-private void appendLedgersMap(BufferedLogChannel entryLogChannel) throws 
IOException {
-long ledgerMapOffset = entryLogChannel.position();
-
-ConcurrentLongLongHashMap ledgersMap = entryLogChannel.getLedgersMap();
-int numberOfLedgers = (int) ledgersMap.size();
-
-// Write the ledgers map into several batches
-
-final int maxMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * LEDGERS_MAP_MAX_BATCH_SIZE;
-final ByteBuf serializedMap = 
ByteBufAllocator.DEFAULT.buffer(maxMapSize);
-
-try {
-ledgersMap.forEach(new BiConsumerLong() {
-int remainingLedgers = numberOfLedgers;
-boolean startNewBatch = true;
-int remainingInBatch = 0;
-
-@Override
-public void accept(long ledgerId, long size) {
-if (startNewBatch) {
-int batchSize = Math.min(remainingLedgers, 
LEDGERS_MAP_MAX_BATCH_SIZE);
-int ledgerMapSize = LEDGERS_MAP_HEADER_SIZE + 
LEDGERS_MAP_ENTRY_SIZE * batchSize;
-
-serializedMap.clear();
-serializedMap.writeInt(ledgerMapSize - 4);
-serializedMap.writeLong(INVALID_LID);
-serializedMap.writeLong(LEDGERS_MAP_ENTRY_ID);
-serializedMap.writeInt(batchSize);
-
-startNewBatch = false;
-remainingInBatch = batchSize;
-}
-// Dump the ledger in the current batch
-serializedMap.writeLong(ledgerId);
-serializedMap.writeLong(size);
---remainingLedgers;
-
-if (--remainingInBatch == 0) {
-// Close current batch
-try {
-entryLogChannel.write(serializedMap);
-} catch (IOException e) {
-throw new RuntimeException(e);
-}
-
-startNewBatch = true;
-}
-}
-});
-} catch (RuntimeException e) {
-if (e.getCause() instanceof IOException) {
-throw (IOException) e.getCause();
-} else {
-throw e;
-}
-} finally {
-serializedMap.release();
-}
-// Flush the ledger's map out before we write the header.
-// Otherwise the header might point to something that is not fully 
written
-entryLogChannel.flush();
-
-// Update the headers with the map offset and count of ledgers
-ByteBuffer mapInfo = ByteBuffer.allocate(8 + 4);
-mapInfo.putLong(ledgerMapOffset);
-mapInfo.putInt(numberOfLedgers);
-mapInfo.flip();
-entryLogChannel.fileChannel.write(mapInfo, 
LEDGERS_MAP_OFFSET_POSITION);
-}
 
 /**
  * An allocator pre-allocates entry log files.
  */
 class EntryLoggerAllocator {
 
 private long preallocatedLogId;
-private Future preallocation = null;
+Future preallocation = null;
 private ExecutorService allocatorExecutor;
-private final Object createEntryLogLock = new Object();
-private final Object createCompactionLogLock = new Object();
 
 EntryLoggerAllocator(long logId) {
 preallocatedLogId = logId;
 allocatorExecutor = Executors.newSingleThreadExecutor();
 }
 
-BufferedLogChannel createNewLog() throws IOException {
-synchronized (createEntryLogLock) {
-BufferedLogChannel bc;
-if (!entryLogPreAllocationEnabled){
-// create a new log directly
-bc = allocateNewLog();
-return bc;
-} else {
-// allocate directly to response request
-if (null == preallocation){
-bc = allocateNewLog();
+synchronized long getPreallocatedLogId(){
+return preallocatedLogId;
+}
+
+synchronized BufferedLogChannel createNewLog() throws IOException {
+BufferedLogChannel bc;
+if (!entryLogPreAllocationEnabled || null == 

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180505208
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -92,13 +99,10 @@
 private final long logId;
 private final EntryLogMetadata entryLogMetadata;
 private final File logFile;
+private Long ledgerIdAssigned = UNASSIGNED_LEDGERID;
 
 Review comment:
   use `long`


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on a change in pull request #1281: Issue #570: Introducing 
EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#discussion_r180505355
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -84,6 +88,9 @@
  */
 public class EntryLogger {
 private static final Logger LOG = 
LoggerFactory.getLogger(EntryLogger.class);
+static final Long UNASSIGNED_LEDGERID = Long.valueOf(-1);
 
 Review comment:
   `long` please. 
   
   `long UNASSIGNED_LEDGERID = -1L`


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] reddycharan commented on issue #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

reddycharan commented on issue #1281: Issue #570: Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#issuecomment-380157141
 
 
   Hey @sijie I think I addressed all of your comments and tests are looking 
good.


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:
us...@infra.apache.org


With regards,
Apache Git Services

Jenkins build became unstable: bookkeeper_postcommit_master_java9 #97

[Apache Jenkins Server]

See 

Jenkins build is back to stable : bookkeeper_release_branch #115

[Apache Jenkins Server]

See 

Jenkins build is back to stable : bookkeeper_postcommit_master_java8 #97

[Apache Jenkins Server]

See 

[GitHub] eolivelli closed pull request #1327: Update News after release of 4.6.2

[GitBox]

eolivelli closed pull request #1327: Update News after release of 4.6.2
URL: https://github.com/apache/bookkeeper/pull/1327
 
 
   

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/site/releases.md b/site/releases.md
index 20bcb7d65..142b98aa2 100644
--- a/site/releases.md
+++ b/site/releases.md
@@ -62,6 +62,15 @@ Client Guide | API docs
 
 ## News
 
+### 9 April, 2018: Release 4.6.2 available
+
+This is the ninth release of BookKeeper as an Apache Top Level Project!
+
+The 4.6.2 release is a bugfix release.
+
+See [BookKeeper 4.6.2 Release Notes](../docs/4.6.2/overview/releaseNotes) for 
details.
+
+
 ### 30 January, 2018: Release 4.6.1 available
 
 This is the eighth release of BookKeeper as an Apache Top Level Project!


 


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli commented on issue #1327: Update News after release of 4.6.2

[GitBox]

eolivelli commented on issue #1327: Update News after release of 4.6.2
URL: https://github.com/apache/bookkeeper/pull/1327#issuecomment-380040916
 
 
   @sijie could you please merge this one ?


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli commented on issue #1327: Update News after release of 4.6.2

[GitBox]

eolivelli commented on issue #1327: Update News after release of 4.6.2
URL: https://github.com/apache/bookkeeper/pull/1327#issuecomment-380038769
 
 
   @sijie I did the change very quickly, I must have missed one key :-) thank 
you


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] eolivelli opened a new pull request #1327: Update News after release of 4.6.2

[GitBox]

eolivelli opened a new pull request #1327: Update News after release of 4.6.2
URL: https://github.com/apache/bookkeeper/pull/1327
 
 
   


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie closed pull request #1325: Update missing configuration settings & missing shell commands

[GitBox]

sijie closed pull request #1325: Update missing configuration settings & 
missing shell commands
URL: https://github.com/apache/bookkeeper/pull/1325
 
 
   

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/conf/bk_server.conf b/conf/bk_server.conf
index 8016a7867..96ddcbca7 100755
--- a/conf/bk_server.conf
+++ b/conf/bk_server.conf
@@ -349,10 +349,28 @@ ledgerDirectories=/tmp/bk-data
 # Directories to store index files. If not specified, will use 
ledgerDirectories to store.
 # indexDirectories=/tmp/bk-data
 
-# Minimum safe Usable size to be available in index directory for bookie to 
create
+# Minimum safe usable size to be available in index directory for bookie to 
create
 # Index File while replaying journal at the time of bookie Start in Readonly 
Mode (in bytes)
 # minUsableSizeForIndexFileCreation=1073741824
 
+# Minimum safe usable size to be available in ledger directory for bookie to 
create
+# entry log files (in bytes).
+#
+# This parameter allows creating entry log files when there are enough disk 
spaces, even when
+# the bookie is running at readonly mode because of the disk usage is 
exceeding `diskUsageThreshold`.
+# Because compaction, journal replays can still write data to disks when a 
bookie is readonly.
+# 
+# Default value is 1.2 * `logSizeLimit`.
+#
+# minUsableSizeForEntryLogCreation=
+
+# Minimum safe usable size to be available in ledger directory for bookie to 
accept
+# high priority writes even it is in readonly mode.
+#
+# If not set, it is the value of `minUsableSizeForEntryLogCreation`
+#
+# minUsableSizeForHighPriorityWrites=
+
 # When entryLogPerLedgerEnabled is enabled, checkpoint doesn't happens
 # when a new active entrylog is created / previous one is rolled over.
 # Instead SyncThread checkpoints periodically with 'flushInterval' delay
diff --git a/site/_data/cli/shell.yaml b/site/_data/cli/shell.yaml
index 6edc2e665..092c8b126 100644
--- a/site/_data/cli/shell.yaml
+++ b/site/_data/cli/shell.yaml
@@ -6,7 +6,7 @@ commands:
 description: Enable autorecovery of underreplicated ledgers
   - flag: -disable
 description: Disable autorecovery of underreplicated ledgers
-- name: bookieFormat
+- name: bookieformat
   description: Format the current server contents.
   options:
   - flag: -nonInteractive
@@ -15,6 +15,13 @@ commands:
 description: If [nonInteractive] is specified, then whether to force 
delete the old data without prompt..?
   - flag: -deleteCookie
 description: Delete its cookie on zookeeper
+- name: initbookie
+  description: |
+Initialize new bookie, by making sure that the journalDir, ledgerDirs and
+indexDirs are empty and there is no registered Bookie with this BookieId.
+
+If there is data present in current bookie server, the init operation will 
fail. If you want to format
+the bookie server, use `bookieformat`.
 - name: bookieinfo
   description: Retrieve bookie info such as free and total disk space.
 - name: bookiesanity
@@ -79,13 +86,30 @@ commands:
 description: Bookie Id of missing replica
   - flag: -excludingmissingreplica N
 description: Bookie Id of missing replica to ignore
+  - flag: -printmissingreplica
+description: Whether to print missingreplicas list?
 - name: metaformat
-  description: Format Bookkeeper metadata in Zookeeper.
+  description: |
+Format Bookkeeper metadata in Zookeeper. This command is deprecated since 
4.7.0,
+in favor of using `initnewcluster` for initializing a new cluster and 
`nukeexistingcluster` for nuking an existing cluster.
   options:
   - flag: -nonInteractive
 description: Whether to confirm if old data exists..?
   - flag: -force
 description: If [nonInteractive] is specified, then whether to force 
delete the old data without prompt.
+- name: initnewcluster
+  description: |
+Initializes a new bookkeeper cluster. If initnewcluster fails then try 
nuking
+existing cluster by running nukeexistingcluster before running 
initnewcluster again
+- name: nukeexistingcluster
+  description: Nuke bookkeeper cluster by deleting metadata
+  options:
+  - flag: -zkledgersrootpath
+description: zookeeper ledgers rootpath
+  - flag: -instanceid
+description: instance id
+  - flag: -force
+description: If instanceid is not specified, then whether to force nuke 
the metadata without validating instanceid
 - name: lostbookierecoverydelay
   description: Setter and Getter for LostBookieRecoveryDelay value (in 
seconds) in Zookeeper.
   options:
@@ -154,3 +178,13 @@ commands:
 description: Print status of the ledger updation (default false)
   - flag: -printprogress N
 description: Print messages on every configured seconds if verbose turned 
on (default 10 secs)
+- name: whoisauditor
+  description: Print 

[GitHub] sijie commented on issue #1325: Update missing configuration settings & missing shell commands

[GitBox]

sijie commented on issue #1325: Update missing configuration settings & missing 
shell commands
URL: https://github.com/apache/bookkeeper/pull/1325#issuecomment-380033571
 
 
   IGNORE CI


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie commented on issue #1325: Update missing configuration settings & missing shell commands

[GitBox]

sijie commented on issue #1325: Update missing configuration settings & missing 
shell commands
URL: https://github.com/apache/bookkeeper/pull/1325#issuecomment-380033520
 
 
   ignore ci, since this change only touch configuration file and website, and 
travis CI passed.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie opened a new pull request #1326: Prepare 4.7.0 release

[GitBox]

sijie opened a new pull request #1326: Prepare 4.7.0 release
URL: https://github.com/apache/bookkeeper/pull/1326
 
 
   
   
   Descriptions of the changes in this PR:
   
   - copy the current `docs/latest` as `docs/4.7.0`
   - bump latest development release to 4.8.0 SNAPSHOT release
   
   beside that, also change stable version to 4.6.2 release
   
   NOTE: this change doesn't put 4.7.0 in the menu. will update menu and 
releases page after 4.7.0 is released


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie closed pull request #1324: Remove WriteFlags from public API

[GitBox]

sijie closed pull request #1324: Remove WriteFlags from public API
URL: https://github.com/apache/bookkeeper/pull/1324
 
 
   

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/client/LedgerCreateOp.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java
index aa6cd69d0..d6c77de3b 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerCreateOp.java
@@ -21,8 +21,10 @@
 
 package org.apache.bookkeeper.client;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.security.GeneralSecurityException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
@@ -238,7 +240,7 @@ private void createComplete(int rc, LedgerHandle lh) {
 cb.createComplete(rc, lh, ctx);
 }
 
-static class CreateBuilderImpl implements CreateBuilder {
+public static class CreateBuilderImpl implements CreateBuilder {
 
 private final BookKeeper bk;
 private int builderEnsembleSize = 3;
@@ -260,12 +262,15 @@ public CreateBuilder withEnsembleSize(int ensembleSize) {
 return this;
 }
 
-@Override
 public CreateBuilder withWriteFlags(EnumSet writeFlags) {
 this.builderWriteFlags = writeFlags;
 return this;
 }
 
+public CreateBuilder withWriteFlags(WriteFlag... writeFlags) {
+return withWriteFlags(EnumSet.copyOf(Arrays.asList(writeFlags)));
+}
+
 @Override
 public CreateBuilder withWriteQuorumSize(int writeQuorumSize) {
 this.builderWriteQuorumSize = writeQuorumSize;
@@ -278,6 +283,7 @@ public CreateBuilder withAckQuorumSize(int ackQuorumSize) {
 return this;
 }
 
+@SuppressFBWarnings("EI_EXPOSE_REP2")
 @Override
 public CreateBuilder withPassword(byte[] password) {
 this.builderPassword = password;
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/CreateBuilder.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/CreateBuilder.java
index aa6ad6d4b..cd5fd3d7c 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/CreateBuilder.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/api/CreateBuilder.java
@@ -20,8 +20,6 @@
  */
 package org.apache.bookkeeper.client.api;
 
-import java.util.Arrays;
-import java.util.EnumSet;
 import java.util.Map;
 import org.apache.bookkeeper.common.annotation.InterfaceAudience.Public;
 import org.apache.bookkeeper.common.annotation.InterfaceStability.Unstable;
@@ -93,26 +91,6 @@
  */
 CreateBuilder withDigestType(DigestType digestType);
 
-/**
- * Set write flags. Write wlags specify the behaviour of writes
- *
- * @param writeFlags the flags
- *
- * @return the builder itself
- */
-CreateBuilder withWriteFlags(EnumSet writeFlags);
-
-/**
- * Set write flags. Write wlags specify the behaviour of writes
- *
- * @param writeFlags the flags
- *
- * @return the builder itself
- */
-default CreateBuilder withWriteFlags(WriteFlag ... writeFlags) {
-return withWriteFlags(EnumSet.copyOf(Arrays.asList(writeFlags)));
-}
-
 /**
  * Switch the ledger into 'Advanced' mode. A ledger used in Advanced mode 
will explicitly generate the sequence of
  * entry identifiers. Advanced ledgers can be created with a client side 
defined ledgerId
diff --git 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
index 5efd7bde7..f7ff4365f 100644
--- 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
+++ 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
@@ -48,7 +48,7 @@
 
 import org.apache.bookkeeper.client.BKException.BKDigestMatchException;
 import org.apache.bookkeeper.client.BKException.Code;
-import org.apache.bookkeeper.client.api.CreateBuilder;
+import org.apache.bookkeeper.client.LedgerCreateOp.CreateBuilderImpl;
 import org.apache.bookkeeper.client.api.DeleteBuilder;
 import org.apache.bookkeeper.client.api.OpenBuilder;
 import org.apache.bookkeeper.common.util.OrderedExecutor;
@@ -209,7 +209,7 @@ protected void setBookkeeperConfig(ClientConfiguration 
config) {
 when(bk.getConf()).thenReturn(config);
 }
 
-protected CreateBuilder newCreateLedgerOp() {
+protected 

[GitHub] sijie commented on issue #1325: Update missing configuration settings & missing shell commands

[GitBox]

sijie commented on issue #1325: Update missing configuration settings & missing 
shell commands
URL: https://github.com/apache/bookkeeper/pull/1325#issuecomment-379995170
 
 
   This PR was based on the change at #1324 . 4d6e1ab is the change for review.


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:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] sijie opened a new pull request #1325: Update missing configuration settings & missing shell commands

[GitBox]

sijie opened a new pull request #1325: Update missing configuration settings & 
missing shell commands
URL: https://github.com/apache/bookkeeper/pull/1325
 
 
   Descriptions of the changes in this PR:
   
   Update missing configuration settings & missing shell commands for preparing 
4.7.0 release.


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:
us...@infra.apache.org


With regards,
Apache Git Services