[GitHub] sijie opened a new pull request #245: Update README status about distributedlog project

[GitBox]

sijie opened a new pull request #245: Update README status about distributedlog 
project
URL: https://github.com/apache/distributedlog/pull/245
 
 
   Descriptions of the changes in this PR:
   
   As distributedlog modules are merged into Apache BookKeeper, update the 
`README` status to reflect the project status.
   
   See 
[BP-26](http://bookkeeper.apache.org/bps/BP-26-move-distributedlog-core-library/)
 for current status.
   


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 #1294: BK configuration file updates

[GitBox]

sijie commented on issue #1294: BK configuration file updates
URL: https://github.com/apache/bookkeeper/pull/1294#issuecomment-376337056
 
 
   
https://sijie.github.io/bookkeeper-staging-site/docs/latest/reference/config/ 
=> This is how the configuration settings are organized 


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 #1294: BK configuration file updates

[GitBox]

sijie opened a new pull request #1294: BK configuration file updates
URL: https://github.com/apache/bookkeeper/pull/1294
 
 
   Descriptions of the changes in this PR:
   
   - Change two default values: `fileInfoCacheInitialCapacity` to 1/4 of 
openFileLimit() and `journalRemoveFromPageCache` to true.
   - Fix default values in conf/bk_server.conf to make them consistent with 
`ServerConfiguration`
   - Organize the settings into sections for both `conf/bk_server.conf` and 
`site/_data/config/bk_server.yaml`


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 #1283: Issue #1282: call to appendLedgersMap in flushCompactionLog

[GitBox]

sijie commented on issue #1283: Issue #1282: call to appendLedgersMap in 
flushCompactionLog
URL: https://github.com/apache/bookkeeper/pull/1283#issuecomment-376330512
 
 
   @reddycharan did you look at @yzang 's comment?


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_r177236857
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -264,6 +274,8 @@ public EntryLogger(ServerConfiguration conf,
 // but the protocol varies so an exact value is difficult to determine
 this.maxSaneEntrySize = conf.getNettyMaxFrameSizeBytes() - 500;
 this.ledgerDirsManager = ledgerDirsManager;
+this.conf = conf;
+entryLogPerLedgerEnabled = conf.isEntryLogPerLedgerEnabled();
 
 Review comment:
   hmm..I considered moving EntryLogManager, EntryLogManagerForSingleEntryLog 
and EntryLogManagerForEntryLogPerLedger out off EntryLogger, but I realized 
that there is circular dependency. EntryLogger has a EntryLogManager reference, 
and for certain functionalities of EntryLogManager, it need to have reference 
of EntryLogger to call its methods in EntryLogManager. So I don’t see much 
benefits other than just creating new files and new classes (which I'm ok with 
if it is must 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 issue #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

sijie commented on issue #1281: Issue #570: Introducing EntryLogManager.
URL: https://github.com/apache/bookkeeper/pull/1281#issuecomment-376311538
 
 
   @ivankelly : 
   
   > Move into its own java file. EntryLogger is already huge.
   > Move into it's own file.
   
   
   I see you left a few comments around moving entry log manager out side of 
the entry logger class. It is a bit difficult in this patch, because entry log 
manager is sharing common stuffs (such as buffered log channels) between 
implementations. 
   
   I would suggest not doing this large refactor (by moving stuffs out of entry 
logger). Let this PR focus on the interface abstraction and make EntryLogger 
responsible for managing EntryLogManager. Once we agree on the interface, we 
can do the cleanup by moving the interface out and have some common abstract 
implementation in subsequent changes.
   
   Let's focus on abstracting interface at this PR, not to complicate stuffs.


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_r177174411
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -513,78 +519,86 @@ 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();
+private void appendLedgersMap(Long ledgerId) throws IOException {
 
 Review comment:
   will move this appendLedgersMap method to BufferedLogChannel. Because of 
that explicit need of converting from ledgerid to logchannel and 
synchronization wont be required.


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 #1283: Issue #1282: call to appendLedgersMap in flushCompactionLog

[GitBox]

reddycharan commented on issue #1283: Issue #1282: call to appendLedgersMap in 
flushCompactionLog
URL: https://github.com/apache/bookkeeper/pull/1283#issuecomment-376239538
 
 
   @ivankelly updated PR description.


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 #82

[Apache Jenkins Server]

See 

Jenkins build became unstable: bookkeeper_release_branch #100

[Apache Jenkins Server]

See 

[GitHub] ivankelly commented on a change in pull request #1293: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox]

ivankelly commented on a change in pull request #1293: Implement directly 
ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1293#discussion_r177057026
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
 ##
 @@ -355,122 +356,127 @@ private static ByteBuf serializeProtobuf(MessageLite 
msg, ByteBufAllocator alloc
 }
 
 @Sharable
-public static class RequestEncoder extends MessageToMessageEncoder 
{
+public static class RequestEncoder extends ChannelOutboundHandlerAdapter {
 
-final EnDecoder REQ_PREV3;
-final EnDecoder REQ_V3;
+final EnDecoder reqPreV3;
+final EnDecoder reqV3;
 
 public RequestEncoder(ExtensionRegistry extensionRegistry) {
-REQ_PREV3 = new RequestEnDeCoderPreV3(extensionRegistry);
-REQ_V3 = new RequestEnDecoderV3(extensionRegistry);
+reqPreV3 = new RequestEnDeCoderPreV3(extensionRegistry);
+reqV3 = new RequestEnDecoderV3(extensionRegistry);
 }
-
+
 @Override
-protected void encode(ChannelHandlerContext ctx, Object msg, 
List out) throws Exception {
+public void write(ChannelHandlerContext ctx, Object msg, 
ChannelPromise promise) throws Exception {
+if (LOG.isTraceEnabled()) {
+LOG.trace("Encode request {} to channel {}.", msg, 
ctx.channel());
+}
 if (msg instanceof BookkeeperProtocol.Request) {
-out.add(REQ_V3.encode(msg, ctx.alloc()));
+ctx.write(reqV3.encode(msg, ctx.alloc()), promise);
 } else if (msg instanceof BookieProtocol.Request) {
-out.add(REQ_PREV3.encode(msg, ctx.alloc()));
+ctx.write(reqPreV3.encode(msg, ctx.alloc()), promise);
 } else {
 LOG.error("Invalid request to encode to {}: {}", 
ctx.channel(), msg.getClass().getName());
-out.add(msg);
+ctx.write(msg, promise);
 }
 }
 }
 
 @Sharable
-public static class RequestDecoder extends MessageToMessageDecoder 
{
-final EnDecoder REQ_PREV3;
-final EnDecoder REQ_V3;
+public static class RequestDecoder extends ChannelInboundHandlerAdapter {
+final EnDecoder reqPreV3;
+final EnDecoder reqV3;
 boolean usingV3Protocol;
 
 RequestDecoder(ExtensionRegistry extensionRegistry) {
-REQ_PREV3 = new RequestEnDeCoderPreV3(extensionRegistry);
-REQ_V3 = new RequestEnDecoderV3(extensionRegistry);
+reqPreV3 = new RequestEnDeCoderPreV3(extensionRegistry);
+reqV3 = new RequestEnDecoderV3(extensionRegistry);
 usingV3Protocol = true;
 }
 
 @Override
-protected void decode(ChannelHandlerContext ctx, Object msg, 
List out) throws Exception {
-if (LOG.isDebugEnabled()) {
-LOG.debug("Received request {} from channel {} to decode.", 
msg, ctx.channel());
+public void channelRead(ChannelHandlerContext ctx, Object msg) throws 
Exception {
+if (LOG.isTraceEnabled()) {
+LOG.trace("Received request {} from channel {} to decode.", 
msg, ctx.channel());
 }
-if (!(msg instanceof ByteBuf)) {
-out.add(msg);
-return;
-}
-ByteBuf buffer = (ByteBuf) msg;
-buffer.markReaderIndex();
-
-if (usingV3Protocol) {
-try {
-out.add(REQ_V3.decode(buffer));
-} catch (InvalidProtocolBufferException e) {
-usingV3Protocol = false;
-buffer.resetReaderIndex();
-out.add(REQ_PREV3.decode(buffer));
+try {
+if (!(msg instanceof ByteBuf)) {
+LOG.error("Received invalid request {} from channel {} to 
decode.", msg, ctx.channel());
+ctx.fireChannelRead(msg);
+return;
 }
-} else {
-out.add(REQ_PREV3.decode(buffer));
+ByteBuf buffer = (ByteBuf) msg;
+buffer.markReaderIndex();
+Object result;
+if (usingV3Protocol) {
+try {
+result = reqV3.decode(buffer);
+} catch (InvalidProtocolBufferException e) {
+usingV3Protocol = false;
+buffer.resetReaderIndex();
+result = reqPreV3.decode(buffer);
+}
+} else {
+result = reqPreV3.decode(buffer);
+}
+ctx.fireChannelRead(result);
+} finally {
+ReferenceCountUtil.release(msg);
 }
 }
 }
 
-

[GitHub] ivankelly commented on issue #589: command line options

[GitBox]

ivankelly commented on issue #589: command line options
URL: https://github.com/apache/bookkeeper/issues/589#issuecomment-376128524
 
 
   @sijie this will be handled automatically by jcommander in the new CLI, 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] ivankelly closed issue #588: Bk vs gluster docs

[GitBox]

ivankelly closed issue #588: Bk vs gluster docs
URL: https://github.com/apache/bookkeeper/issues/588
 
 
   


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] ivankelly commented on issue #588: Bk vs gluster docs

[GitBox]

ivankelly commented on issue #588: Bk vs gluster docs
URL: https://github.com/apache/bookkeeper/issues/588#issuecomment-376128353
 
 
   Won't 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] ivankelly closed issue #585: make pushing a docker image as part of the release procedure

[GitBox]

ivankelly closed issue #585: make pushing a docker image as part of the release 
procedure
URL: https://github.com/apache/bookkeeper/issues/585
 
 
   


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] ivankelly commented on issue #585: make pushing a docker image as part of the release procedure

[GitBox]

ivankelly commented on issue #585: make pushing a docker image as part of the 
release procedure
URL: https://github.com/apache/bookkeeper/issues/585#issuecomment-376128240
 
 
   Already part of the Release guide.


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] ivankelly commented on issue #587: Re-replicator: bookie checking should handle flapping bookie registration

[GitBox]

ivankelly commented on issue #587: Re-replicator: bookie checking should handle 
flapping bookie registration
URL: https://github.com/apache/bookkeeper/issues/587#issuecomment-376127971
 
 
   Implemented in #58 


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] ivankelly closed issue #587: Re-replicator: bookie checking should handle flapping bookie registration

[GitBox]

ivankelly closed issue #587: Re-replicator: bookie checking should handle 
flapping bookie registration
URL: https://github.com/apache/bookkeeper/issues/587
 
 
   


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] ivankelly closed issue #584: Jenkins failed while test BookieClientTest.testWriteGaps

[GitBox]

ivankelly closed issue #584: Jenkins failed while test 
BookieClientTest.testWriteGaps
URL: https://github.com/apache/bookkeeper/issues/584
 
 
   


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] ivankelly commented on issue #584: Jenkins failed while test BookieClientTest.testWriteGaps

[GitBox]

ivankelly commented on issue #584: Jenkins failed while test 
BookieClientTest.testWriteGaps
URL: https://github.com/apache/bookkeeper/issues/584#issuecomment-376126324
 
 
   Closing. 


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 #577: Clean up internal Bookkeeper constructors

[GitBox]

eolivelli commented on issue #577: Clean up internal Bookkeeper constructors
URL: https://github.com/apache/bookkeeper/issues/577#issuecomment-376126101
 
 
   We could deprecate current constructors in 4.8.
   Once we are OK with new API


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] ivankelly commented on issue #578: make MajorCompaction and MinorCompactions controlled by at least time of the day/day of the week

[GitBox]

ivankelly commented on issue #578: make MajorCompaction and MinorCompactions 
controlled by at least time of the day/day of the week
URL: https://github.com/apache/bookkeeper/issues/578#issuecomment-376126018
 
 
   This is being implemented in #851


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] ivankelly commented on issue #577: Clean up internal Bookkeeper constructors

[GitBox]

ivankelly commented on issue #577: Clean up internal Bookkeeper constructors
URL: https://github.com/apache/bookkeeper/issues/577#issuecomment-376125719
 
 
   This should be done as part of new api work. Ideally there should be no 
constructor for the BookKeeper object, but everything should go through a 
Builder that returns a o.a.b.client.api.BookKeeper object. 
   
   Currently it's not really useful to have o.a.b.client.api.BookKeeper as you 
have to import o.a.b.client.BookKeeper to create one.
   
   ClientConfiguration should probably be deprecated too, and all the 
configuration in it put in the builder.
   
   (cc @sijie @eolivelli )


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] ivankelly closed issue #567: Make bookie automatically create folders on new machine.

[GitBox]

ivankelly closed issue #567: Make bookie automatically create folders on new 
machine.
URL: https://github.com/apache/bookkeeper/issues/567
 
 
   


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] ivankelly commented on issue #567: Make bookie automatically create folders on new machine.

[GitBox]

ivankelly commented on issue #567: Make bookie automatically create folders on 
new machine.
URL: https://github.com/apache/bookkeeper/issues/567#issuecomment-376124249
 
 
   Closing this as it's not a feature we will implement.


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 #1293: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox]

eolivelli opened a new pull request #1293: Implement directly 
ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1293
 
 
   This change is mostly a clean up/refactor which drops intermediate 
MessageToMessageEncoder and MessageToMessageDecoder from BookieProtoEncoding
   
   This is a manual cherry pick of #1286 
   


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] ivankelly commented on issue #1283: Issue #1282: call to appendLedgersMap in flushCompactionLog

[GitBox]

ivankelly commented on issue #1283: Issue #1282: call to appendLedgersMap in 
flushCompactionLog
URL: https://github.com/apache/bookkeeper/pull/1283#issuecomment-376113964
 
 
   retest this please


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] ivankelly commented on issue #1283: Issue #1282: call to appendLedgersMap in flushCompactionLog

[GitBox]

ivankelly commented on issue #1283: Issue #1282: call to appendLedgersMap in 
flushCompactionLog
URL: https://github.com/apache/bookkeeper/pull/1283#issuecomment-376112510
 
 
   @reddycharan will you update the PR description with the description from 
the issue, so we'll be able to see the reason for the change directly from the 
git log?


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] ivankelly commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

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

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -264,6 +274,8 @@ public EntryLogger(ServerConfiguration conf,
 // but the protocol varies so an exact value is difficult to determine
 this.maxSaneEntrySize = conf.getNettyMaxFrameSizeBytes() - 500;
 this.ledgerDirsManager = ledgerDirsManager;
+this.conf = conf;
+entryLogPerLedgerEnabled = conf.isEntryLogPerLedgerEnabled();
 
 Review comment:
   The entryLogManager should be constructed outside of the EntryLogger and 
passed in as an argument. The entryLogger shouldn't need to know which 
implementation of the entryLogManager it is using, which is the whole point of 
having an interface. Also injecting the implementation like this, will make for 
easier testing, and SortedLedgerStorage would not need to be touched at all 
(i.e. SortedLedgerStorage can be changed to always create the SingleLog 
variant).


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] ivankelly commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

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

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -802,88 +876,217 @@ private long readLastLogId(File f) {
 }
 }
 
+interface EntryLogManager {
+/*
+ * acquire lock for this ledger.
+ */
+void acquireLock(Long ledgerId);
+
+/*
+ * acquire lock for this ledger if it is not already available for this
+ * ledger then it will create a new one and then acquire lock.
+ */
+void acquireLockByCreatingIfRequired(Long ledgerId);
+
+/*
+ * release lock for this ledger
+ */
+void releaseLock(Long ledgerId);
+
+/*
+ * sets the logChannel for the given ledgerId. The previous one will be
+ * removed from replicaOfCurrentLogChannels. Previous logChannel will 
be
+ * added to rotatedLogChannels.
+ */
+void setCurrentLogForLedger(Long ledgerId, BufferedLogChannel 
logChannel);
+
+/*
+ * gets the logChannel for the given ledgerId.
+ */
+BufferedLogChannel getCurrentLogForLedger(Long ledgerId);
+
+/*
+ * gets the copy of rotatedLogChannels
+ */
+Set getCopyOfRotatedLogChannels();
+
+/*
+ * gets the copy of replicaOfCurrentLogChannels
+ */
+Set getCopyOfCurrentLogs();
+
+/*
+ * gets the active logChannel with the given entryLogId. null if it is
+ * not existing.
+ */
+BufferedLogChannel getCurrentLogIfPresent(long entryLogId);
+
+/*
+ * removes the logChannel from rotatedLogChannels collection
+ */
+void removeFromRotatedLogChannels(BufferedLogChannel 
rotatedLogChannelToRemove);
+
+/*
+ * 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;
+}
+
+class EntryLogManagerForSingleEntryLog implements EntryLogManager {
 
 Review comment:
   Move into it's own file.


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] ivankelly commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

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

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -802,88 +876,217 @@ private long readLastLogId(File f) {
 }
 }
 
+interface EntryLogManager {
 
 Review comment:
   Move into its own java file. EntryLogger is already huge.


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] ivankelly commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

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

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -292,12 +304,37 @@ public EntryLogger(ServerConfiguration conf,
 logId = lastLogId;
 }
 }
-this.leastUnflushedLogId = logId + 1;
+this.recentlyCreatedEntryLogsStatus = new RecentEntryLogsStatus(logId 
+ 1);
 
 Review comment:
   Should be part of the 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] ivankelly commented on a change in pull request #1281: Issue #570: Introducing EntryLogManager.

[GitBox]

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

 ##
 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:
   Separate change, so should not be in this patch.


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 closed pull request #1286: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox]

eolivelli closed pull request #1286: Implement directly 
ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1286
 
 
   

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/proto/BookieProtoEncoding.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
index a45f94eaa..151a799f5 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
@@ -32,12 +32,13 @@
 import io.netty.buffer.Unpooled;
 import io.netty.channel.ChannelHandler.Sharable;
 import io.netty.channel.ChannelHandlerContext;
-import io.netty.handler.codec.MessageToMessageDecoder;
-import io.netty.handler.codec.MessageToMessageEncoder;
+import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.ChannelOutboundHandlerAdapter;
+import io.netty.channel.ChannelPromise;
+import io.netty.util.ReferenceCountUtil;
 
 import java.io.IOException;
 import java.security.NoSuchAlgorithmException;
-import java.util.List;
 
 import org.apache.bookkeeper.proto.BookieProtocol.PacketHeader;
 import org.apache.bookkeeper.proto.checksum.MacDigestManager;
@@ -371,7 +372,7 @@ private static ByteBuf serializeProtobuf(MessageLite msg, 
ByteBufAllocator alloc
  * A request message encoder.
  */
 @Sharable
-public static class RequestEncoder extends MessageToMessageEncoder 
{
+public static class RequestEncoder extends ChannelOutboundHandlerAdapter {
 
 final EnDecoder reqPreV3;
 final EnDecoder reqV3;
@@ -382,17 +383,17 @@ public RequestEncoder(ExtensionRegistry 
extensionRegistry) {
 }
 
 @Override
-protected void encode(ChannelHandlerContext ctx, Object msg, 
List out) throws Exception {
+public void write(ChannelHandlerContext ctx, Object msg, 
ChannelPromise promise) throws Exception {
 if (LOG.isTraceEnabled()) {
 LOG.trace("Encode request {} to channel {}.", msg, 
ctx.channel());
 }
 if (msg instanceof BookkeeperProtocol.Request) {
-out.add(reqV3.encode(msg, ctx.alloc()));
+ctx.write(reqV3.encode(msg, ctx.alloc()), promise);
 } else if (msg instanceof BookieProtocol.Request) {
-out.add(reqPreV3.encode(msg, ctx.alloc()));
+ctx.write(reqPreV3.encode(msg, ctx.alloc()), promise);
 } else {
 LOG.error("Invalid request to encode to {}: {}", 
ctx.channel(), msg.getClass().getName());
-out.add(msg);
+ctx.write(msg, promise);
 }
 }
 }
@@ -401,7 +402,7 @@ protected void encode(ChannelHandlerContext ctx, Object 
msg, List out) t
  * A request message decoder.
  */
 @Sharable
-public static class RequestDecoder extends MessageToMessageDecoder 
{
+public static class RequestDecoder extends ChannelInboundHandlerAdapter {
 final EnDecoder reqPreV3;
 final EnDecoder reqV3;
 boolean usingV3Protocol;
@@ -413,27 +414,33 @@ protected void encode(ChannelHandlerContext ctx, Object 
msg, List out) t
 }
 
 @Override
-protected void decode(ChannelHandlerContext ctx, Object msg, 
List out) throws Exception {
+public void channelRead(ChannelHandlerContext ctx, Object msg) throws 
Exception {
 if (LOG.isTraceEnabled()) {
 LOG.trace("Received request {} from channel {} to decode.", 
msg, ctx.channel());
 }
-if (!(msg instanceof ByteBuf)) {
-out.add(msg);
-return;
-}
-ByteBuf buffer = (ByteBuf) msg;
-buffer.markReaderIndex();
-
-if (usingV3Protocol) {
-try {
-out.add(reqV3.decode(buffer));
-} catch (InvalidProtocolBufferException e) {
-usingV3Protocol = false;
-buffer.resetReaderIndex();
-out.add(reqPreV3.decode(buffer));
+try {
+if (!(msg instanceof ByteBuf)) {
+LOG.error("Received invalid request {} from channel {} to 
decode.", msg, ctx.channel());
+ctx.fireChannelRead(msg);
+return;
 }
-} else {
-out.add(reqPreV3.decode(buffer));
+ByteBuf buffer = (ByteBuf) msg;
+buffer.markReaderIndex();
+Object result;
+if (usingV3Protocol) {
+try {
+   

[GitHub] ivankelly commented on a change in pull request #1292: Avoid acquiring closeLock.readLock() on every add/read operation

[GitBox]

ivankelly commented on a change in pull request #1292: Avoid acquiring 
closeLock.readLock() on every add/read operation
URL: https://github.com/apache/bookkeeper/pull/1292#discussion_r177016874
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java
 ##
 @@ -189,40 +189,35 @@ public PerChannelBookieClientPool 
lookupClient(BookieSocketAddress addr) {
 
 public void writeLac(final BookieSocketAddress addr, final long ledgerId, 
final byte[] masterKey,
 final long lac, final ByteBufList toSend, final WriteLacCallback 
cb, final Object ctx) {
-closeLock.readLock().lock();
-try {
-final PerChannelBookieClientPool client = lookupClient(addr);
-if (client == null) {
-
cb.writeLacComplete(getRc(BKException.Code.BookieHandleNotAvailableException),
-  ledgerId, addr, ctx);
-return;
-}
+final PerChannelBookieClientPool client = lookupClient(addr);
+if (client == null) {
+
cb.writeLacComplete(getRc(BKException.Code.BookieHandleNotAvailableException),
+  ledgerId, addr, ctx);
+return;
+}
 
-toSend.retain();
-client.obtain(new GenericCallback() {
-@Override
-public void operationComplete(final int rc, 
PerChannelBookieClient pcbc) {
-if (rc != BKException.Code.OK) {
-try {
-executor.submitOrdered(ledgerId, new 
SafeRunnable() {
-@Override
-public void safeRun() {
-cb.writeLacComplete(rc, ledgerId, addr, 
ctx);
-}
-});
-} catch (RejectedExecutionException re) {
-
cb.writeLacComplete(getRc(BKException.Code.InterruptedException), ledgerId, 
addr, ctx);
-}
-} else {
-pcbc.writeLac(ledgerId, masterKey, lac, toSend, cb, 
ctx);
+toSend.retain();
+client.obtain(new GenericCallback() {
 
 Review comment:
   nit: these can become lambdas


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] ivankelly commented on a change in pull request #1292: Avoid acquiring closeLock.readLock() on every add/read operation

[GitBox]

ivankelly commented on a change in pull request #1292: Avoid acquiring 
closeLock.readLock() on every add/read operation
URL: https://github.com/apache/bookkeeper/pull/1292#discussion_r177016752
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java
 ##
 @@ -371,37 +361,32 @@ public void recycle() {
 
 public void readLac(final BookieSocketAddress addr, final long ledgerId, 
final ReadLacCallback cb,
 final Object ctx) {
-closeLock.readLock().lock();
-try {
-final PerChannelBookieClientPool client = lookupClient(addr);
-if (client == null) {
-
cb.readLacComplete(getRc(BKException.Code.BookieHandleNotAvailableException), 
ledgerId, null, null,
-ctx);
-return;
-}
-client.obtain(new GenericCallback() {
-@Override
-public void operationComplete(final int rc, 
PerChannelBookieClient pcbc) {
-if (rc != BKException.Code.OK) {
-try {
-executor.submitOrdered(ledgerId, new 
SafeRunnable() {
-@Override
-public void safeRun() {
-cb.readLacComplete(rc, ledgerId, null, 
null, ctx);
-}
-});
-} catch (RejectedExecutionException re) {
-
cb.readLacComplete(getRc(BKException.Code.InterruptedException),
-ledgerId, null, null, ctx);
-}
-return;
+final PerChannelBookieClientPool client = lookupClient(addr);
+if (client == null) {
+
cb.readLacComplete(getRc(BKException.Code.BookieHandleNotAvailableException), 
ledgerId, null, null,
+ctx);
+return;
+}
+client.obtain(new GenericCallback() {
+@Override
+public void operationComplete(final int rc, PerChannelBookieClient 
pcbc) {
+if (rc != BKException.Code.OK) {
+try {
+executor.submitOrdered(ledgerId, new SafeRunnable() {
+@Override
+public void safeRun() {
+cb.readLacComplete(rc, ledgerId, null, null, 
ctx);
+}
+});
+} catch (RejectedExecutionException re) {
+
cb.readLacComplete(getRc(BKException.Code.InterruptedException),
+ledgerId, null, null, ctx);
 }
-pcbc.readLac(ledgerId, cb, ctx);
+return;
 
 Review comment:
   nit: remove this.


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 #1286: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox]

eolivelli commented on issue #1286: Implement directly 
ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1286#issuecomment-376063680
 
 
   I am going to merge this PR and cherry pick to 4.6 today, as soon as CI ends 
with success


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 #1286: Implement directly ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder

[GitBox]

eolivelli commented on issue #1286: Implement directly 
ChannelOutboundHandlerAdapter in BookieProtoEncoding#ResponseEncoder
URL: https://github.com/apache/bookkeeper/pull/1286#issuecomment-376063582
 
 
   retest this please


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