Jenkins build became unstable: distributedlog-nightly-build #534

2018-01-02 Thread Apache Jenkins Server
See 




[GitHub] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159362343
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##
 @@ -512,9 +515,11 @@ private BookKeeper(ClientConfiguration conf,
 // initialize ledger manager
 try {
 this.ledgerManagerFactory =
-LedgerManagerFactory.newLedgerManagerFactory(conf, 
((ZKRegistrationClient) regClient).getZk());
+LedgerManagerFactory.newLedgerManagerFactory(conf, 
regClient.getLayoutManager());
 } catch (KeeperException ke) {
 
 Review comment:
   right, will 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159362065
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListUnderReplicatedLedgerService.java
 ##
 @@ -51,12 +49,12 @@
 static final Logger LOG = 
LoggerFactory.getLogger(ListUnderReplicatedLedgerService.class);
 
 protected ServerConfiguration conf;
-protected ZooKeeper zk;
+protected LayoutManager layoutManager;
 
-public ListUnderReplicatedLedgerService(ServerConfiguration conf, 
ZooKeeper zk) {
+public ListUnderReplicatedLedgerService(ServerConfiguration conf, 
LayoutManager layoutManager) {
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159362034
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
 ##
 @@ -245,7 +245,14 @@ public void checkpointComplete(Checkpoint checkPoint, 
boolean compact)
 InterleavedLedgerStorage storage = new InterleavedLedgerStorage();
 storage.initialize(
 conf,
-LedgerManagerFactory.newLedgerManagerFactory(conf, 
zkc).newLedgerManager(),
+LedgerManagerFactory
+.newLedgerManagerFactory(
+conf,
+new ZkLayoutManager(
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159362068
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
 ##
 @@ -122,7 +120,6 @@ public void testMissingLogId() throws Exception {
 Bookie.checkDirectoryStructure(curDir);
 
 ServerConfiguration conf = 
TestBKConfiguration.newServerConfiguration();
-conf.setJournalDirName(tmpDir.toString());
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159362004
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java
 ##
 @@ -79,6 +83,13 @@ private BKHttpServiceProvider(BookieServer bookieServer,
 this.bookieServer = bookieServer;
 this.autoRecovery = autoRecovery;
 this.serverConf = serverConf;
+this.layoutManager = new ZkLayoutManager(
 
 Review comment:
   Thanks, will try 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361986
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
 ##
 @@ -134,7 +136,9 @@ private void initialize(ServerConfiguration conf, 
ZooKeeper zkc)
 throws UnavailableException {
 try {
 LedgerManagerFactory ledgerManagerFactory = LedgerManagerFactory
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361963
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/RegistrationManager.java
 ##
 @@ -104,4 +105,27 @@ RegistrationManager initialize(ServerConfiguration conf,
  * @throws BookieException when fail to remove cookie
  */
 void removeCookie(String bookieId, Version version) throws BookieException;
+
+/**
+ * Gets layout manager.
+ *
+ * @return the layout manager
+ */
+LayoutManager getLayoutManager();
+
+/**
+ * Prepare ledgers root node, availableNode, readonly node..
+ *
+ * @param conf the conf
+ * @return Returns true if old data exists, false if not.
+ */
+boolean prepareFormat(ServerConfiguration conf) throws Exception;
+
+/**
+ * Do format boolean.
+ *
+ * @param conf the conf
+ * @return Returns true if success do format, false if not.
+ */
+boolean doFormat(ServerConfiguration conf) throws Exception;
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361978
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtoEncoding.java
 ##
 @@ -416,8 +413,8 @@ protected void encode(ChannelHandlerContext ctx, Object 
msg, List out) t
 
 @Override
 protected void decode(ChannelHandlerContext ctx, Object msg, 
List out) throws Exception {
-if (LOG.isTraceEnabled()) {
-LOG.trace("Received request {} from channel {} to decode.", 
msg, ctx.channel());
+if (LOG.isDebugEnabled()) {
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361972
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
 ##
 @@ -322,9 +322,7 @@ public void processResult(int rc, String path, Object ctx) 
{
 @Override
 public void registerLedgerMetadataListener(long ledgerId, 
LedgerMetadataListener listener) {
 if (null != listener) {
-if (LOG.isDebugEnabled()) {
-LOG.debug("Registered ledger metadata listener {} on ledger 
{}.", listener, ledgerId);
-}
+LOG.debug("Registered ledger metadata listener {} on ledger {}.", 
listener, ledgerId);
 
 Review comment:
   Thanks, will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361947
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/RegistrationClient.java
 ##
 @@ -116,4 +116,7 @@ RegistrationClient initialize(ClientConfiguration conf,
  * @param listener listener to receive the topology changes of bookies.
  */
 void unwatchReadOnlyBookies(RegistrationListener listener);
+
+// TODO:
 
 Review comment:
   Thanks will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361860
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##
 @@ -512,9 +515,11 @@ private BookKeeper(ClientConfiguration conf,
 // initialize ledger manager
 try {
 this.ledgerManagerFactory =
-LedgerManagerFactory.newLedgerManagerFactory(conf, 
((ZKRegistrationClient) regClient).getZk());
+LedgerManagerFactory.newLedgerManagerFactory(conf, 
regClient.getLayoutManager());
 } catch (KeeperException ke) {
 
 Review comment:
   thanks. will change 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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361825
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
 ##
 @@ -459,8 +460,8 @@ public void run() {
 numReqInLastForceWrite = 0;
 }
 }
+numReqInLastForceWrite += 
req.process(shouldForceWrite);
 
 Review comment:
   will change it, Seems it is from merge.


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] zhaijack commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack commented on a change in pull request #936: Issue 897: un-bind 
zookeeper from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159361792
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
 ##
 @@ -686,21 +688,25 @@ int runCmd(CommandLine cmdLine) throws Exception {
 predicate = replicasList -> 
!replicasList.contains(excludingBookieId);
 }
 
-ZooKeeper zk = null;
+LedgerManagerFactory mFactory = null;
 try {
-zk = ZooKeeperClient.newBuilder()
-.connectString(bkConf.getZkServers())
-.sessionTimeoutMs(bkConf.getZkTimeout())
-.build();
-LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(bkConf, zk);
+mFactory = LedgerManagerFactory.newLedgerManagerFactory(
+bkConf,
+new ZkLayoutManager(
 
 Review comment:
   Thanks, will try 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 opened a new pull request #937: Issue 935: Issue 915: Part 3 - Make circe-checksum module work for both java 8 and java 9

2018-01-02 Thread GitBox
sijie opened a new pull request #937: Issue 935: Issue 915: Part 3 - Make 
circe-checksum module work for both java 8 and java 9
URL: https://github.com/apache/bookkeeper/pull/937
 
 
   Descriptions of the changes in this PR:
   
   - switch from jmockit to mockito, and from testng to junit. make testing 
and mocking framework consistent across modules.
   - skip `-Werror` check in circe-checksum since `finalize` is deprecated 
at java 9
   - change CRC32CDigestManager to use `circe-checksum` module
   - fix shading to include `circe-checksum`
   
   This change is based #917 . Gitsha 73049f8 contains the changes to make 
circe-checksum module work for both java8 and java9.
   


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159357085
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
 ##
 @@ -245,7 +245,14 @@ public void checkpointComplete(Checkpoint checkPoint, 
boolean compact)
 InterleavedLedgerStorage storage = new InterleavedLedgerStorage();
 storage.initialize(
 conf,
-LedgerManagerFactory.newLedgerManagerFactory(conf, 
zkc).newLedgerManager(),
+LedgerManagerFactory
+.newLedgerManagerFactory(
+conf,
+new ZkLayoutManager(
 
 Review comment:
   try to new layout manager from registration manager


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159357040
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListUnderReplicatedLedgerService.java
 ##
 @@ -51,12 +49,12 @@
 static final Logger LOG = 
LoggerFactory.getLogger(ListUnderReplicatedLedgerService.class);
 
 protected ServerConfiguration conf;
-protected ZooKeeper zk;
+protected LayoutManager layoutManager;
 
-public ListUnderReplicatedLedgerService(ServerConfiguration conf, 
ZooKeeper zk) {
+public ListUnderReplicatedLedgerService(ServerConfiguration conf, 
LayoutManager layoutManager) {
 
 Review comment:
   Take LedgerManagerFactory from AutoRecovery


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356528
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/DefaultSpeculativeRequestExecutionPolicy.java
 ##
 @@ -38,7 +38,7 @@
  * are between {@code firstSpeculativeRequestTimeout} and {@code 
maxSpeculativeRequestTimeout}.
  */
 public class DefaultSpeculativeRequestExecutionPolicy implements 
SpeculativeRequestExecutionPolicy {
-private static final Logger LOG = 
LoggerFactory.getLogger(DefaultSpeculativeRequestExecutionPolicy.class);
+private static final Logger LOG = 
LoggerFactory.getLogger(PendingReadOp.class);
 
 Review comment:
   I think this change is not needed. bad merge?


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356321
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
 ##
 @@ -512,9 +515,11 @@ private BookKeeper(ClientConfiguration conf,
 // initialize ledger manager
 try {
 this.ledgerManagerFactory =
-LedgerManagerFactory.newLedgerManagerFactory(conf, 
((ZKRegistrationClient) regClient).getZk());
+LedgerManagerFactory.newLedgerManagerFactory(conf, 
regClient.getLayoutManager());
 } catch (KeeperException ke) {
 
 Review comment:
   if we remove zk, `LedgerManagerFactory.newLedgerManagerFactory` should not 
throw KeeperException.


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356702
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/AbstractZkLedgerManager.java
 ##
 @@ -322,9 +322,7 @@ public void processResult(int rc, String path, Object ctx) 
{
 @Override
 public void registerLedgerMetadataListener(long ledgerId, 
LedgerMetadataListener listener) {
 if (null != listener) {
-if (LOG.isDebugEnabled()) {
-LOG.debug("Registered ledger metadata listener {} on ledger 
{}.", listener, ledgerId);
-}
+LOG.debug("Registered ledger metadata listener {} on ledger {}.", 
listener, ledgerId);
 
 Review comment:
   bad merge here?


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356237
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
 ##
 @@ -459,8 +460,8 @@ public void run() {
 numReqInLastForceWrite = 0;
 }
 }
+numReqInLastForceWrite += 
req.process(shouldForceWrite);
 
 Review comment:
   I think the changes in this file are not needed. probably come from merge?
 


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356917
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
 ##
 @@ -134,7 +136,9 @@ private void initialize(ServerConfiguration conf, 
ZooKeeper zkc)
 throws UnavailableException {
 try {
 LedgerManagerFactory ledgerManagerFactory = LedgerManagerFactory
 
 Review comment:
   I would suggest getting layout manager from registration  client, rather 
than construct zk layout manager directly.


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356213
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
 ##
 @@ -842,15 +848,18 @@ public int runCmd(CommandLine cmdLine) throws Exception {
 return -1;
 }
 
-ZooKeeper zk = null;
 LedgerManagerFactory mFactory = null;
 LedgerManager m = null;
 try {
-zk = ZooKeeperClient.newBuilder()
-.connectString(bkConf.getZkServers())
-.sessionTimeoutMs(bkConf.getZkTimeout())
-.build();
-mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(bkConf, zk);
+mFactory = LedgerManagerFactory.newLedgerManagerFactory(
+bkConf,
+new ZkLayoutManager(
 
 Review comment:
   try to avoid using ZkLayoutManager directly


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159357018
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/ListLedgerService.java
 ##
 @@ -53,12 +53,12 @@
 static final Logger LOG = LoggerFactory.getLogger(ListLedgerService.class);
 
 protected ServerConfiguration conf;
-protected ZooKeeper zk;
+protected LayoutManager layoutManager;
 
-public ListLedgerService(ServerConfiguration conf, ZooKeeper zk) {
+public ListLedgerService(ServerConfiguration conf, LayoutManager 
layoutManager) {
 
 Review comment:
   This service should just take LedgerManagerFactory from bookie.


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356554
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/RegistrationClient.java
 ##
 @@ -116,4 +116,7 @@ RegistrationClient initialize(ClientConfiguration conf,
  * @param listener listener to receive the topology changes of bookies.
  */
 void unwatchReadOnlyBookies(RegistrationListener listener);
+
+// TODO:
 
 Review comment:
   remove TODO and add javadoc?


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356594
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/RegistrationManager.java
 ##
 @@ -104,4 +105,27 @@ RegistrationManager initialize(ServerConfiguration conf,
  * @throws BookieException when fail to remove cookie
  */
 void removeCookie(String bookieId, Version version) throws BookieException;
+
+/**
+ * Gets layout manager.
+ *
+ * @return the layout manager
+ */
+LayoutManager getLayoutManager();
+
+/**
+ * Prepare ledgers root node, availableNode, readonly node..
+ *
+ * @param conf the conf
+ * @return Returns true if old data exists, false if not.
+ */
+boolean prepareFormat(ServerConfiguration conf) throws Exception;
+
+/**
+ * Do format boolean.
+ *
+ * @param conf the conf
+ * @return Returns true if success do format, false if not.
+ */
+boolean doFormat(ServerConfiguration conf) throws Exception;
 
 Review comment:
   rename this to `format`.


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356972
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java
 ##
 @@ -79,6 +83,13 @@ private BKHttpServiceProvider(BookieServer bookieServer,
 this.bookieServer = bookieServer;
 this.autoRecovery = autoRecovery;
 this.serverConf = serverConf;
+this.layoutManager = new ZkLayoutManager(
 
 Review comment:
   Can you get the layout manager from bookieServer?


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356478
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
 ##
 @@ -1130,120 +1121,40 @@ public void processResult(int rc, String s, Object 
ctx) {
  */
 public static boolean format(ClientConfiguration conf,
 boolean isInteractive, boolean force) throws Exception {
-ZooKeeper zkc = ZooKeeperClient.newBuilder()
-.connectString(conf.getZkServers())
-.sessionTimeoutMs(conf.getZkTimeout())
-.build();
-BookKeeper bkc = null;
-try {
-boolean ledgerRootExists = null != zkc.exists(
-conf.getZkLedgersRootPath(), false);
-boolean availableNodeExists = null != zkc.exists(
-conf.getZkAvailableBookiesPath(), false);
-List zkAcls = ZkUtils.getACLs(conf);
-// Create ledgers root node if not exists
-if (!ledgerRootExists) {
-zkc.create(conf.getZkLedgersRootPath(), "".getBytes(UTF_8),
-zkAcls, CreateMode.PERSISTENT);
-}
-// create available bookies node if not exists
-if (!availableNodeExists) {
-zkc.create(conf.getZkAvailableBookiesPath(), 
"".getBytes(UTF_8),
-zkAcls, CreateMode.PERSISTENT);
-}
 
-// create readonly bookies node if not exists
-if (null == zkc.exists(conf.getZkAvailableBookiesPath() + "/" + 
READONLY, false)) {
-zkc.create(
-conf.getZkAvailableBookiesPath() + "/" + READONLY,
-new byte[0],
-zkAcls,
-CreateMode.PERSISTENT);
-}
+try (RegistrationManager rm = new ZKRegistrationManager()) {
 
 Review comment:
   Better load registration manager from configuration rather than constructing 
directly from ZKRegistrationManager? Change `format(ClientConfiguration ...)` 
to `format(ServerConfiguration ..)`, because registration manager is only 
available in ServerConfiguration. This format method is only called by 
BookieShell.


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159356203
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
 ##
 @@ -686,21 +688,25 @@ int runCmd(CommandLine cmdLine) throws Exception {
 predicate = replicasList -> 
!replicasList.contains(excludingBookieId);
 }
 
-ZooKeeper zk = null;
+LedgerManagerFactory mFactory = null;
 try {
-zk = ZooKeeperClient.newBuilder()
-.connectString(bkConf.getZkServers())
-.sessionTimeoutMs(bkConf.getZkTimeout())
-.build();
-LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(bkConf, zk);
+mFactory = LedgerManagerFactory.newLedgerManagerFactory(
+bkConf,
+new ZkLayoutManager(
 
 Review comment:
   I think it would be better to avoid construct ZkLayoutManager directly.
   
   A better way to do this:
   
   ```
   try (RegistrationManager rm = ...) {
   try (LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(.., rm.getLayoutManager()) {
  ...
   }
   }
   ```


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159357005
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/GetLedgerMetaService.java
 ##
 @@ -45,12 +48,12 @@
 static final Logger LOG = 
LoggerFactory.getLogger(GetLedgerMetaService.class);
 
 protected ServerConfiguration conf;
-protected ZooKeeper zk;
-
-public GetLedgerMetaService(ServerConfiguration conf, ZooKeeper zk) {
+protected LayoutManager layoutManager;
+// TODO: remove zk?
+public GetLedgerMetaService(ServerConfiguration conf, LayoutManager 
layoutManager) {
 
 Review comment:
   This service should just take LedgerManagerFactory from Bookie


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 #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
sijie commented on a change in pull request #936: Issue 897: un-bind zookeeper 
from bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936#discussion_r159357105
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
 ##
 @@ -122,7 +120,6 @@ public void testMissingLogId() throws Exception {
 Bookie.checkDirectoryStructure(curDir);
 
 ServerConfiguration conf = 
TestBKConfiguration.newServerConfiguration();
-conf.setJournalDirName(tmpDir.toString());
 
 Review comment:
   bad merge in this 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] athanatos commented on a change in pull request #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
athanatos commented on a change in pull request #851: Issue 578 : make 
MajorCompaction controlled by time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#discussion_r159351938
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
 ##
 @@ -264,12 +278,53 @@ public void safeRun() {
 // collection cycle started
 forceGarbageCollection.set(false);
 }
+
+}
+
+/**
+ * increase the threshold when the system in low load status.
+ */
+private void activateMedianThreshold(){
+majorCompactionThreshold = medianMajorCompactionThreshold;
+}
+/**
+ * increase the threshold when the system in a very low load status.
+ */
+private void activateHighThreshold(){
+majorCompactionThreshold = highMajorCompactionThreshold;
+}
+
+/**
+ * Check whether the configured cron is met and activate threshold.
+ */
+private void changeMajorCompactionThreshold(){
 
 Review comment:
   This strikes me as quite brittle.  It seems to me that you'll end up 
selecting the median (or high) threshhold several times in a row (approximately 
majorCompactionInterval / gcWaitTIme times).  It sort of works out because at 
line 350 all but 1 (or possibly 0 or 2, I'm not sure that the time between 
calls to now() couldn't cause a race condition where you skip a high/median 
compaction threshold or do it twice) will fail the curTime - 
lastMajorCompactionTime > majorCompactionInterval test.
   
   It would be considerably simpler if you changed 
changeMajorCompactionThreshold to return the current threshold, move the call 
under the test at line 350, and maintain two pieces of state recording the last 
cron time where you returned a high or median value.


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] athanatos commented on a change in pull request #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
athanatos commented on a change in pull request #851: Issue 578 : make 
MajorCompaction controlled by time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#discussion_r159350527
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
 ##
 @@ -68,9 +69,16 @@
 long lastMinorCompactionTime;
 
 boolean enableMajorCompaction = false;
-final double majorCompactionThreshold;
+final double originalMajorCompactionThreshold;
+double majorCompactionThreshold;
 
 Review comment:
   I don't think you want to maintain both majorCompactionThreshold and 
originalMajorCompactionThreshold.  majorCompactionThreshold is mainly used in 
two places:
   1. In the constructor, you do some validation of the config, but you don't 
need to change it to do that.
   2. In runWithFlags, you set it based on the current time, pass it into 
doCompactEntryLogs, and restore it back to original before returning.
   
   Instead, rename originalMajorCompactionThreshold to majorCompactionThreshold 
(and leave it final) and get rid of the mutable majorCompactionThreshold.  In 
runWithFlags, modify changeMajorCompactionThreshold to 
getCurrentMajorCompactionThreshold and let it return a value based on the 
current time which you can keep in a local in runWithFlags.  You don't need 
restoreThreshold at all.
 


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] zhaijack opened a new pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin

2018-01-02 Thread GitBox
zhaijack opened a new pull request #936: Issue 897: un-bind zookeeper from 
bookkeeper admin
URL: https://github.com/apache/bookkeeper/pull/936
 
 
   Descriptions of the changes in this PR:
   - remove zookeeper from LedgerLayout to make LedgerLayout as a pure layout 
object,
   - add LayoutManager, move zookeeper related utils into ZKLayoutManager for 
storing/reading/delete layout from zookeeper,
   - unbind zookeeper from LedgerManagerFactory, use LayoutManager instead,
   - unbind zookeeper from bookkeeperadmin.format,
   - fix compile and test errors.
   
   
   Master Issue: #897
   
   > ---
   > Be sure to do all of the following to help us incorporate your contribution
   > quickly and easily:
   >
   > - [x] Make sure the PR title is formatted like:
   > `: Description of pull request`
   > `e.g. Issue 123: Description ...`
   > `e.g. BOOKKEEPER-1234: Description ...`
   > - [x] Make sure tests pass via `mvn clean apache-rat:check install 
spotbugs:check`.
   > - [x] Replace `` in the title with the actual 
Issue/JIRA 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] sijie opened a new issue #935: org.apache.bookkeeper.tests.shaded.BookKeeperServerShadedArtifactTest.testProtobufIsShaded failed

2018-01-02 Thread GitBox
sijie opened a new issue #935: 
org.apache.bookkeeper.tests.shaded.BookKeeperServerShadedArtifactTest.testProtobufIsShaded
 failed
URL: https://github.com/apache/bookkeeper/issues/935
 
 
   
   **BUG REPORT**
   
   1. Please describe the issue you observed:
   
   - What did you do?
   
   run `mvn install` in current master
   
   - What did you expect to see?
   
   it should complete successfully.
   
   - What did you see instead?
   
   The command failed with 
   
   ```
   Error Message
   Expected exception: java.lang.ClassNotFoundException
   Stacktrace
   java.lang.AssertionError: Expected exception: 
java.lang.ClassNotFoundException
   ```
   
   The problem is introduced due to merging #856 after merging #928. Because 
`pulsar-checksum` depends on guava, which is not included in the changes of 
#928. 


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 #888: Rework of binary distribution licenses

2018-01-02 Thread GitBox
sijie commented on a change in pull request #888: Rework of binary distribution 
licenses
URL: https://github.com/apache/bookkeeper/pull/888#discussion_r159347761
 
 

 ##
 File path: bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt
 ##
 @@ -202,507 +202,265 @@
limitations under the License.
 
 

-For lib/org.slf4j-slf4j-api-1.7.25.jar
-and lib/org.slf4j-slf4j-log4j12-1.7.25.jar
-
-Copyright (c) 2004-2017 QOS.ch
-All rights reserved.
-
-Permission is hereby granted, free  of charge, to any person obtaining
-a  copy  of this  software  and  associated  documentation files  (the
-"Software"), to  deal in  the Software without  restriction, including
-without limitation  the rights to  use, copy, modify,  merge, publish,
-distribute,  sublicense, and/or sell  copies of  the Software,  and to
-permit persons to whom the Software  is furnished to do so, subject to
-the following conditions:
-
-The  above  copyright  notice  and  this permission  notice  shall  be
-included in all copies or substantial portions of the Software.
-
-THE  SOFTWARE IS  PROVIDED  "AS  IS", WITHOUT  WARRANTY  OF ANY  KIND,
-EXPRESS OR  IMPLIED, INCLUDING  BUT NOT LIMITED  TO THE  WARRANTIES OF
-MERCHANTABILITY,FITNESSFORA   PARTICULARPURPOSEAND
-NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
-LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
-OF CONTRACT, TORT OR OTHERWISE,  ARISING FROM, OUT OF OR IN CONNECTION
-WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+The following bundled 3rd party jars are distributed under the
+Apache Software License, Version 2.
+
+- lib/com.fasterxml.jackson.core-jackson-annotations-2.8.9.jar [1]
+- lib/com.fasterxml.jackson.core-jackson-core-2.8.9.jar [2]
+- lib/com.fasterxml.jackson.core-jackson-databind-2.8.9.jar [3]
+- lib/com.fasterxml.jackson.module-jackson-module-paranamer-2.8.4.jar [4]
+- lib/com.fasterxml.jackson.module-jackson-module-scala_2.11-2.8.4.jar [5]
+- lib/com.google.guava-guava-20.0.jar [6]
+- lib/commons-cli-commons-cli-1.2.jar [7]
+- lib/commons-codec-commons-codec-1.6.jar [8]
+- lib/commons-collections-commons-collections-3.2.2.jar [9]
+- lib/commons-configuration-commons-configuration-1.10.jar [10]
+- lib/commons-io-commons-io-2.4.jar [11]
+- lib/commons-lang-commons-lang-2.6.jar [12]
+- lib/commons-logging-commons-logging-1.1.1.jar [13]
+- lib/com.twitter-finagle-base-http_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-core_2.11-6.34.0.jar [15]
+- lib/com.twitter-finagle-http_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-http2_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-netty4_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-netty4-http_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-thrift_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-toggle_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-tunable_2.11-6.44.0.jar [14]
+- lib/com.twitter-finagle-zipkin-core_2.11-6.44.0.jar [14]
+- lib/com.twitter-libthrift-0.5.0-7.jar [16]
+- lib/com.twitter-scrooge-core_2.11-4.16.0.jar [17]
+- lib/com.twitter-twitter-server_2.11-1.29.0.jar [18]
+- lib/com.twitter-util-app_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-cache_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-codec_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-collection_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-core_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-events_2.11-6.43.0.jar [20]
+- lib/com.twitter-util-function_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-hashing_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-jvm_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-lint_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-logging_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-registry_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-stats_2.11-6.33.0.jar [19]
+- lib/com.twitter-util-tunable_2.11-6.43.0.jar [20]
+- lib/io.dropwizard.metrics-metrics-core-3.1.0.jar [21]
+- lib/io.dropwizard.metrics-metrics-graphite-3.1.0.jar [21]
+- lib/io.dropwizard.metrics-metrics-jvm-3.1.0.jar [21]
+- lib/io.netty-netty-3.10.1.Final.jar [22]
+- lib/io.netty-netty-all-4.1.12.Final.jar [23]
+- lib/io.prometheus-simpleclient-0.0.21.jar [24]
+- lib/io.prometheus-simpleclient_common-0.0.21.jar [24]
+- lib/io.prometheus-simpleclient_hotspot-0.0.21.jar [24]
+- lib/io.prometheus-simpleclient_servlet-0.0.21.jar [24]
+- lib/io.vertx-vertx-auth-common-3.4.1.jar [25]
+- lib/io.vertx-vertx-core-3.4.1.jar [26]
+- lib/io.vertx-vertx-web-3.4.1.jar [27]
+- lib/javax.inject-javax.inject-1.jar [28]
+- lib/log4j-log4j-1.2.15.jar [29]
+- lib/net.java.dev.jna-jna-3.2.7.jar [30]
+- lib/org.apache.commons-commons-collections4-4.1.jar [31]
+- lib/org.apache.commons-commons-lang3-3.3.2.jar [32]
+- lib/org.apache.zookeeper-zookeeper-3.5.3-beta.jar [33]
+- lib/org.eclipse.jetty-jetty-http-9.4.5.v20170502.jar [34]
+- lib/org.eclipse.jetty-jetty-io-9.4.5.v20170502.jar [34]
+- 

[GitHub] sijie commented on a change in pull request #832: Issue 620: Close the fileChannels for read when they are idle

2018-01-02 Thread GitBox
sijie commented on a change in pull request #832: Issue 620: Close the 
fileChannels for read when they are idle
URL: https://github.com/apache/bookkeeper/pull/832#discussion_r159347662
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
 ##
 @@ -330,54 +334,111 @@ private int readFromLogChannel(long entryLogId, 
BufferedReadChannel channel, Byt
  * A thread-local variable that wraps a mapping of log ids to 
bufferedchannels
  * These channels should be used only for reading. logChannel is the one
  * that is used for writes.
+ * We use this Guava cache to store the BufferedReadChannel.
+ * When the BufferedReadChannel is removed, the underlying fileChannel's 
refCnt decrease 1,
+ * temporally use 1h to relax replace after reading.
  */
-private final ThreadLocal> logid2Channel =
-new ThreadLocal>() {
+private final ThreadLocal> logid2Channel =
+new ThreadLocal>() {
 @Override
-public Map initialValue() {
+public Cache initialValue() {
 // Since this is thread local there only one modifier
 // We dont really need the concurrency, but we need to use
 // the weak values. Therefore using the concurrency level of 1
-return new MapMaker().concurrencyLevel(1)
-.weakValues()
-.makeMap();
+return CacheBuilder.newBuilder().concurrencyLevel(1)
+.expireAfterAccess(expireReadChannelCache, 
TimeUnit.MILLISECONDS)
+//decrease the refCnt
+.removalListener(removal -> 
logid2FileChannel.get(removal.getKey()).release())
+.build(readChannelLoader);
 }
 };
 
+// only used for test.
+ThreadLocal> getLogid2Channel() {
 
 Review comment:
   accessing internal state is okay if you are just testing the specific test 
case. I don't see a reason we will change it in future. 
   
   @ArvinDevel can you add `@VisibleForTesting`?


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 #832: Issue 620: Close the fileChannels for read when they are idle

2018-01-02 Thread GitBox
sijie commented on a change in pull request #832: Issue 620: Close the 
fileChannels for read when they are idle
URL: https://github.com/apache/bookkeeper/pull/832#discussion_r159347434
 
 

 ##
 File path: 
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/EntryLogTest.java
 ##
 @@ -315,6 +316,104 @@ public void testRecoverFromLedgersMapOnV0EntryLog() 
throws Exception {
 assertEquals(120, meta.getRemainingSize());
 }
 
+/**
+ * Test Cache for logid2Channel and concurrentMap for logid2FileChannel 
work correctly.
+ * Note that, when an entryLogger is initialized, the entry log id will 
increase one.
+ * when the preallocation is enabled, a new entrylogger will cost 2 logId.
+ */
+@Test
+public void testCacheInEntryLog() throws Exception {
+File tmpDir = createTempDir("bkTest", ".dir");
+File curDir = Bookie.getCurrentDirectory(tmpDir);
+Bookie.checkDirectoryStructure(curDir);
+
+int gcWaitTime = 1000;
+ServerConfiguration conf = 
TestBKConfiguration.newServerConfiguration();
+conf.setGcWaitTime(gcWaitTime);
+conf.setLedgerDirNames(new String[] {tmpDir.toString()});
+//since last access, expire after 1s
+conf.setExpireReadChannelCache(1000);
+conf.setEntryLogFilePreAllocationEnabled(false);
+// below one will cost logId 0
+Bookie bookie = new Bookie(conf);
+// create some entries
+int numLogs = 4;
+int numEntries = 10;
+long[][] positions = new long[numLogs][];
+for (int i = 0; i < numLogs; i++) {
+positions[i] = new long[numEntries];
+EntryLogger logger = new EntryLogger(conf,
+bookie.getLedgerDirsManager());
+for (int j = 0; j < numEntries; j++) {
+positions[i][j] = logger.addEntry(i, generateEntry(i, 
j).nioBuffer());
+}
+logger.flush();
+LOG.info("log id is {}, LeastUnflushedLogId is {} ", 
logger.getCurrentLogId(),
+logger.getLeastUnflushedLogId());
+}
+
+for (int i = 1; i < numLogs + 1; i++) {
+File logFile = new File(curDir, Long.toHexString(i) + ".log");
+assertTrue(logFile.exists());
+}
+
+// create some read for the entry log
+EntryLogger logger = ((InterleavedLedgerStorage) 
bookie.ledgerStorage).entryLogger;
+ThreadLocal>  cacheThreadLocal = 
logger.getLogid2Channel();
+ConcurrentMap 
logid2FileChannel = logger.getLogid2FileChannel();
+for (int j = 0; j < numEntries; j++) {
+logger.readEntry(0, j, positions[0][j]);
+}
+LOG.info("cache size is {}, content is {}", 
cacheThreadLocal.get().size(),
+cacheThreadLocal.get().asMap().toString());
+// the cache has readChannel for 1.log
+assertNotNull(cacheThreadLocal.get().getIfPresent(1L));
+for (int j = 0; j < numEntries; j++) {
+logger.readEntry(1, j, positions[1][j]);
+}
+LOG.info("cache size is {}, content is {}", 
cacheThreadLocal.get().size(),
+cacheThreadLocal.get().asMap().toString());
+// the cache has readChannel for 2.log
+assertNotNull(cacheThreadLocal.get().getIfPresent(2L));
+// expire time
+Thread.sleep(1000);
 
 Review comment:
   @ArvinDevel : guava cache has a mock ticker that allows you to advance time 
to trigger expriation.


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] dlg99 commented on a change in pull request #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
dlg99 commented on a change in pull request #851: Issue 578 : make 
MajorCompaction controlled by time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#discussion_r159347344
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
 ##
 @@ -1101,6 +1105,101 @@ public ServerConfiguration 
setMajorCompactionThreshold(double threshold) {
 return this;
 }
 
+/**
+ * Get threshold of median major compaction.
+ *
+ * same with the @see #getMajorCompactionThreshold()
+ * the high threshold is used when the system is on a low load status
 
 Review comment:
   The change has nothing to do with adjusting compaction parameters based on 
actual system/bookie load as one can think after reading this, it adjusts them 
on schedule (which supposed to reflect one's best guess for the system load). 
I'd adjust the doc text to reflect 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] sijie commented on issue #927: BP-24: BookieScanner: Enhance Data Integrity

2018-01-02 Thread GitBox
sijie commented on issue #927: BP-24: BookieScanner: Enhance Data Integrity
URL: https://github.com/apache/bookkeeper/pull/927#issuecomment-354914980
 
 
   /cc @jvrao for a review, since he was thinking about adding similar features 
before.


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 #927: BP-24: BookieScanner: Enhance Data Integrity

2018-01-02 Thread GitBox
sijie commented on a change in pull request #927: BP-24: BookieScanner: Enhance 
Data Integrity
URL: https://github.com/apache/bookkeeper/pull/927#discussion_r159346937
 
 

 ##
 File path: site/bps/BP-24-BookieScanner.md
 ##
 @@ -0,0 +1,92 @@
+?---
+title: "BP-24: BookieScanner: Enhance Data Integrity"
+issue: https://github.com/apache/bookkeeper/
 
 Review comment:
   @ArvinDevel can you create a master issue?


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] kishorekasi opened a new issue #934: Additional stats in perChannelBookieClient

2018-01-02 Thread GitBox
kishorekasi opened a new issue #934: Additional stats in perChannelBookieClient
URL: https://github.com/apache/bookkeeper/issues/934
 
 
   **FEATURE REQUEST**
   
   1. Please describe the feature you are requesting.
   Additional stats to track outstanding WRITE, READ or ALL operations, time 
taken to connect on a PCBC.
   
   2. Indicate the importance of this issue to you (blocker, must-have, 
should-have, nice-to-have). Are you currently using any workarounds to address 
this issue?
   should-have
   
   


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] kishorekasi opened a new issue #933: Enable PEM format X.509 certs

2018-01-02 Thread GitBox
kishorekasi opened a new issue #933: Enable PEM format X.509 certs
URL: https://github.com/apache/bookkeeper/issues/933
 
 
   **FEATURE REQUEST**
   
   1. Please describe the feature you are requesting.
   Currently Bookkeeper only supports JKS key store and trust stores. Another 
widely used file format is PEM for certificates and certificate chains. Without 
in-build support in bookkeeper, an external tool is necessary to convert PEM 
files to key stores.
   
   2. Indicate the importance of this issue to you (blocker, must-have, 
should-have, nice-to-have). Are you currently using any workarounds to address 
this issue?
   should-have.
   
   3. Provide any additional detail on your proposed use case for this feature.
   Support for PEM file formats in addition to JKS file formats.


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 #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
reddycharan commented on a change in pull request #851: Issue 578 : make 
MajorCompaction controlled by time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#discussion_r159342428
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
 ##
 @@ -1101,6 +1105,101 @@ public ServerConfiguration 
setMajorCompactionThreshold(double threshold) {
 return this;
 }
 
+/**
+ * Get threshold of median major compaction.
+ *
+ * same with the @see #getMajorCompactionThreshold()
+ * the high threshold is used when the system is on a low load status
+ *
+ * @return threshold of median major compaction
+ */
+public double getMedianMajorCompactionThreshold() {
+return getDouble(MAJOR_MEDIAN_COMPACTION_THRESHOLD, 0.7f);
+}
+
+/**
+ * Set threshold of median major compaction.
+ *
+ * @see #getMedianMajorCompactionThreshold()
+ *
+ * @param threshold
+ *  Threshold of median major compaction
+ * @return server configuration
+ */
+public ServerConfiguration setMedianMajorCompactionThreshold(double 
threshold) {
+setProperty(MAJOR_MEDIAN_COMPACTION_THRESHOLD, threshold);
+return this;
+}
+
+/**
+ * Get threshold of high major compaction.
+ *
+ * same with the @see #getMajorCompactionThreshold()
+ * the high threshold is used when the system is on a very low load status
+ *
+ * @return threshold of high major compaction
+ */
+public double getHighMajorCompactionThreshold() {
+return getDouble(MAJOR_HIGH_COMPACTION_THRESHOLD, 0.9f);
 
 Review comment:
   can you add sample values to bk_server.conf


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] athanatos opened a new pull request #932: ISSUE #931,#907: Add option to track task execution time

2018-01-02 Thread GitBox
athanatos opened a new pull request #932: ISSUE #931,#907: Add option to track 
task execution time
URL: https://github.com/apache/bookkeeper/pull/932
 
 
   Fixes a bug in OrderedScheduler introduced in
   e33ec10aa400f32c2e0278c15ea80a0f624e5919 which failed to track execution
   time with some callsand adds an option to enable it in the bookie.  Also
   fix bug with task_queued duration.
   
   Add a simple mock for remembering stats long enough to verify that
   counters are actually used and sensible in unit tests and bake it into
   BookKeeperClusterTestCase so that we can write tests to ensure that the
   stats are actually counted and make sense.  Use said mock to add simple
   tests for top level read and write stats validating this fix.
   
   (@bug W-4276826@)
   (@bug W-4268290@)
   Signed-off-by: Samuel Just 
   
   > ---
   > 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 ...`
   > `e.g. BOOKKEEPER-1234: Description ...`
   > - [ ] Make sure tests pass via `mvn clean apache-rat:check install 
spotbugs:check`.
   > - [ ] Replace `` in the title with the actual 
Issue/JIRA 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] sijie commented on issue #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
sijie commented on issue #851: Issue 578 : make MajorCompaction controlled by 
time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#issuecomment-354891922
 
 
   @dlg99 I see. I think the main reason you dropped the time-based compaction 
is because of multiple entry loggers feature that Charan is working on. But not 
every use cases will move to use the approach. so compaction can not be avoided 
for some of the use cases. it has the value to have time-based compaction, so 
applications can have a better control on when compaction happens.
   
   If you have time to review this, we can try to consolidate the efforts here 
with your efforts, to find a good approach for 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 commented on issue #930: [WIP] Issue #903: Bc tests based on docker

2018-01-02 Thread GitBox
sijie commented on issue #930: [WIP] Issue #903: Bc tests based on docker
URL: https://github.com/apache/bookkeeper/pull/930#issuecomment-354891069
 
 
   the overall framework looks good to me. However I am not sure if using a 
backward compat proxy is the right approach here. 
   
   1) I guess the reason it exists is because you want to simplify writing test 
cases and be able to adopt to multiple versions. But this approach only works 
for very basic bc tests, because you can't really have a common proxy over 
multiple versions. 
   
   2) I am afraid adding a bc proxy will introduce pains on maintaining the 
proxy for bc tests. It doesn't sound actually reduce the maintenance overhead 
here.
   
   3) I would prefer testing BC between client and bookies without extra proxy 
if possible. You can actually test and assert on the client behaviors that 
actually talks to bookies and zookeeper, instead of proxying or delegation.
   
   Other comments:
   
   - I think we need to stop support versions prior to 4.3.0. We need to 
enforce EOL as we planned in 
https://cwiki.apache.org/confluence/display/BOOKKEEPER/BP-13+-+Time+Based+Release+Plan
   - I am not sure whether arquillian is good or not. but the original idea is 
using arquillian is not only for BC tests, but also for integration/failure 
injection tests. It is good for use one framework, rather than inventing our 
own testing wheel for different purposes.


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] dlg99 commented on issue #851: Issue 578 : make MajorCompaction controlled by time of the day/day of the week

2018-01-02 Thread GitBox
dlg99 commented on issue #851: Issue 578 : make MajorCompaction controlled by 
time of the day/day of the week
URL: https://github.com/apache/bookkeeper/pull/851#issuecomment-354881643
 
 
   @sijie I'll take a look at the change.
   
   I did functionally similar change but it ended up being unused due to less 
than predictable schedule in our pre-prod env and work in progress on having 
multiple active ledgers to eliminate compaction completely. 
   We are pretty much planning to drop time-based configuration but if you feel 
that this is helpful to community I can merge/do a pull req for it to the 
community code.
   
   My approach was slightly different:
   
   abstract config allowed one to specify time ranges and any parameter could 
be enabled to make use of it:
   ```text
   ## Some settings support configuration override for specific time range.
  ## For example one can set:
  ## gcWaitTime=180
  ## gcWaitTime@timerange0=90
  ## 
  ## Time ranges defined as:
  ## .start=
  ## .end=
  ## Optional:
  ## .day=
  ##  consists of prefix "timerange" plus numeric id.
  ## Numeric id starts from 0 and incremented by 1. 
  ## Search for ranges stops on the first missing id and/or on the first 
range that misses 
  ## or has wrong value for the time or day.
  ## 
  ## Start and end time defined in ISO_LOCAL_TIME format (24 hr, local time 
zone)
  ## 
  ## Time ranges can overlap because different ranges can be applied to 
different groups of
  ## parameters.
  ## 
  ## Examples:
  ## 
  ## timerange0.start=09:00:00
  ## timerange0.end=17:59:59.999
  ## 
  ## timerange1.start=22:00
  ## timerange1.end=23:00
  ## timerange1.day=Saturday,Sunday
  ## 
  ## minorCompactionThreshold=0.2
  ## minorCompactionThreshold@timerange0=0.1
  ## 
  ## minorCompactionInterval=3600
  ## minorCompactionInterval@timerange0=1800
  ## 
  ## majorCompactionThreshold=0.3
  ## majorCompactionThreshold@timerange0=0.2
  ## majorCompactionThreshold@timerange1=0.5
  ## 
  ## majorCompactionInterval=86400 
  ## majorCompactionInterval@timerange1=1800 
   ```
   
   ```java
   public double getMinorCompactionThreshold(LocalTime now, DayOfWeek day) {
   return 
getDouble(this.getPropertyNameForFirstActiveTimerange(MINOR_COMPACTION_THRESHOLD,
 now, day), 0.2f);
   }
   ```
   
   and in compaction thread I had to make sure that parameters are refreshed 
before each use like:
   
   ```java
   refreshGcConfigParams(conf.getLocalTime(), conf.getDayOfWeek());
   
   private void refreshGcConfigParams(LocalTime now, DayOfWeek day) {
   gcWaitTime = conf.getGcWaitTime(now, day);
   minorCompactionThreshold = conf.getMinorCompactionThreshold(now, 
day);
   minorCompactionInterval = conf.getMinorCompactionInterval(now, day) 
* SECOND;
   majorCompactionThreshold = conf.getMajorCompactionThreshold(now, 
day);
   majorCompactionInterval = conf.getMajorCompactionInterval(now, day) 
* SECOND;
  .
   ```


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] athanatos opened a new issue #931: Add option for tracking Bookie task execution time

2018-01-02 Thread GitBox
athanatos opened a new issue #931: Add option for tracking Bookie task 
execution time
URL: https://github.com/apache/bookkeeper/issues/931
 
 
   Fix bug in OrderedSafeExecutor which prevents time tracking in some cases 
and add an option to enable on the Bookie.


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 #898: Bookies should prioritize recovery reads/writes

2018-01-02 Thread GitBox
sijie commented on a change in pull request #898: Bookies should prioritize 
recovery reads/writes
URL: https://github.com/apache/bookkeeper/pull/898#discussion_r159286875
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestProcessor.java
 ##
 @@ -152,6 +159,14 @@ public BookieRequestProcessor(ServerConfiguration 
serverCfg, Bookie bookie,
 createExecutor(
 this.serverCfg.getNumLongPollWorkerThreads(),
 "BookieLongPollThread-" + serverCfg.getBookiePort(), 
OrderedScheduler.NO_TASK_LIMIT);
+this.fenceThreadPool = Executors.newCachedThreadPool(
 
 Review comment:
   @merlimat +1 on a separate recovery thread pool.


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 opened a new pull request #930: [WIP] Issue #903: Bc tests based on docker

2018-01-02 Thread GitBox
ivankelly opened a new pull request #930: [WIP] Issue #903: Bc tests based on 
docker
URL: https://github.com/apache/bookkeeper/pull/930
 
 
   This is a work in progress, but I'd like some feedback/opinions before 
cleaning it up.
   
   The basis idea is to generate a proxy server for each client version. Each 
proxy runs in its own container, and has a completely isolated classpath. The 
client is compiled against a single version of BK (4.2.0), so if we have a 
binary break, we should catch it by just using the proxy. This should be tested 
separately though. The proxy server/client is GRpc based.
   
   I've converted one test to show what it looks like in usage. Really, this 
test needs to be broken down into smaller tests, it's a beast right now. This 
all needs a lot of cleanup.
   
   I'm not sure we should use arquillian. It doesn't seem to add much. For all 
the interesting stuff I find myself dropping out to using the docker client 
directly. Arquillian basically just provides something like docker-compose, but 
even then, it's a bit limited.  The documentation is not very good (there's a 
lot of it, but it reads like a novel, and there's no javadoc). It doesn't seem 
to allow you to dump the logs from the containers at the end of a run. I think 
we'd be more productive just using docker-client directly.
   
   


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 #898: Bookies should prioritize recovery reads/writes

2018-01-02 Thread GitBox
ivankelly commented on a change in pull request #898: Bookies should prioritize 
recovery reads/writes
URL: https://github.com/apache/bookkeeper/pull/898#discussion_r159258799
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestProcessor.java
 ##
 @@ -152,6 +159,14 @@ public BookieRequestProcessor(ServerConfiguration 
serverCfg, Bookie bookie,
 createExecutor(
 this.serverCfg.getNumLongPollWorkerThreads(),
 "BookieLongPollThread-" + serverCfg.getBookiePort(), 
OrderedScheduler.NO_TASK_LIMIT);
+this.fenceThreadPool = Executors.newCachedThreadPool(
 
 Review comment:
   +1 on dedicated recovery thread pool. I'm currently working on BC stuff 
first though, but will come back once that's 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,
Apache Git Services


Jenkins build is still unstable: bookkeeper_release_nightly_snapshot #16

2018-01-02 Thread Apache Jenkins Server
See 




Build failed in Jenkins: bookkeeper_postcommit_master_jdkversions » JDK 1.8 (latest),ubuntu #17

2018-01-02 Thread Apache Jenkins Server
See 


Changes:

[mmerli] ISSUE #590 (@bug W-4556980@) Reduce excessive CPU usage on client side:

--
[...truncated 2.06 MB...]
java.lang.NullPointerException
at 
org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorageTest.teardown(DbLedgerStorageTest.java:77)

Running org.apache.bookkeeper.bookie.storage.ldb.WriteCacheTest
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.51 sec - in 
org.apache.bookkeeper.bookie.storage.ldb.WriteCacheTest
Running org.apache.bookkeeper.bookie.storage.ldb.EntryLocationIndexTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.504 sec - in 
org.apache.bookkeeper.bookie.storage.ldb.EntryLocationIndexTest
Running org.apache.bookkeeper.bookie.storage.ldb.ConversionTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.014 sec - in 
org.apache.bookkeeper.bookie.storage.ldb.ConversionTest
Running org.apache.bookkeeper.bookie.storage.ldb.ConversionRollbackTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.799 sec - in 
org.apache.bookkeeper.bookie.storage.ldb.ConversionRollbackTest
Running org.apache.bookkeeper.bookie.BookieThreadTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.193 sec - in 
org.apache.bookkeeper.bookie.BookieThreadTest
Running org.apache.bookkeeper.bookie.TestGcOverreplicatedLedger
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.206 sec - in 
org.apache.bookkeeper.bookie.TestGcOverreplicatedLedger
Running org.apache.bookkeeper.bookie.EntryLogTest
Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.512 sec - in 
org.apache.bookkeeper.bookie.EntryLogTest
Running org.apache.bookkeeper.bookie.LedgerCacheTest
Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 9.262 sec - in 
org.apache.bookkeeper.bookie.LedgerCacheTest
Running org.apache.bookkeeper.bookie.CookieTest
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.861 sec - 
in org.apache.bookkeeper.bookie.CookieTest
Running org.apache.bookkeeper.bookie.EnableZkSecurityBasicTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.947 sec - in 
org.apache.bookkeeper.bookie.EnableZkSecurityBasicTest
Running org.apache.bookkeeper.bookie.BookieShellTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 5.988 sec - in 
org.apache.bookkeeper.bookie.BookieShellTest
Running org.apache.bookkeeper.bookie.BookieInitializationTest
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 33.608 sec - 
in org.apache.bookkeeper.bookie.BookieInitializationTest
Running org.apache.bookkeeper.bookie.BookieJournalNoSyncTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.162 sec - in 
org.apache.bookkeeper.bookie.BookieJournalNoSyncTest
Running org.apache.bookkeeper.bookie.BookieStorageThresholdTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.082 sec - in 
org.apache.bookkeeper.bookie.BookieStorageThresholdTest
Running org.apache.bookkeeper.bookie.TestSkipListArena
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.196 sec - in 
org.apache.bookkeeper.bookie.TestSkipListArena
Running org.apache.bookkeeper.bookie.BookieJournalTest
Tests run: 13, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.546 sec - in 
org.apache.bookkeeper.bookie.BookieJournalTest
Running org.apache.bookkeeper.bookie.UpdateCookieCmdTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 6.948 sec - in 
org.apache.bookkeeper.bookie.UpdateCookieCmdTest
Running org.apache.bookkeeper.bookie.TestSyncThread
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.723 sec - in 
org.apache.bookkeeper.bookie.TestSyncThread
Running org.apache.bookkeeper.bookie.TestEntryMemTable
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.473 sec - in 
org.apache.bookkeeper.bookie.TestEntryMemTable
Running org.apache.bookkeeper.bookie.IndexPersistenceMgrTest
Tests run: 6, Failures: 0, Errors: 6, Skipped: 0, Time elapsed: 0.354 sec <<< 
FAILURE! - in org.apache.bookkeeper.bookie.IndexPersistenceMgrTest
testEvictFileInfoWhenUnderlyingFileDoesntExist(org.apache.bookkeeper.bookie.IndexPersistenceMgrTest)
  Time elapsed: 0.131 sec  <<< ERROR!
org.apache.bookkeeper.bookie.LedgerDirsManager$NoWritableLedgerDirException: 
All ledger directories are non writable
at 
org.apache.bookkeeper.bookie.IndexPersistenceMgrTest.setUp(IndexPersistenceMgrTest.java:76)

testGetFileInfoWriteBeforeRead(org.apache.bookkeeper.bookie.IndexPersistenceMgrTest)
  Time elapsed: 0.004 sec  <<< ERROR!
org.apache.bookkeeper.bookie.LedgerDirsManager$NoWritableLedgerDirException: 
All ledger directories are non writable
at 
org.apache.bookkeeper.bookie.IndexPersistenceMgrTest.setUp(IndexPersistenceMgrTest.java:76)


Build failed in Jenkins: bookkeeper_postcommit_master_jdkversions » JDK 1.9 (latest),ubuntu #17

2018-01-02 Thread Apache Jenkins Server
See 


Changes:

[mmerli] ISSUE #590 (@bug W-4556980@) Reduce excessive CPU usage on client side:

--
[...truncated 2.12 MB...]
WARNING: Illegal reflective access by io.netty.util.internal.ReflectionUtil 
(
 to constructor java.nio.DirectByteBuffer(long,int)
WARNING: Please consider reporting this to the maintainers of 
io.netty.util.internal.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal 
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.032 sec - in 
org.apache.bookkeeper.bookie.BookieStorageThresholdTest
Running org.apache.bookkeeper.bookie.CookieTest
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by com.google.protobuf.UnsafeUtil 
(
 to field java.nio.Buffer.address
WARNING: Please consider reporting this to the maintainers of 
com.google.protobuf.UnsafeUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal 
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 12.824 sec - 
in org.apache.bookkeeper.bookie.CookieTest
Running org.apache.bookkeeper.bookie.TestSyncThread
Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.789 sec - in 
org.apache.bookkeeper.bookie.TestSyncThread
Running org.apache.bookkeeper.bookie.IndexCorruptionTest
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by io.netty.util.internal.ReflectionUtil 
(
 to constructor java.nio.DirectByteBuffer(long,int)
WARNING: Please consider reporting this to the maintainers of 
io.netty.util.internal.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal 
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 72.743 sec - in 
org.apache.bookkeeper.bookie.IndexCorruptionTest
Running org.apache.bookkeeper.bookie.LedgerStorageTest
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by io.netty.util.internal.ReflectionUtil 
(
 to constructor java.nio.DirectByteBuffer(long,int)
WARNING: Please consider reporting this to the maintainers of 
io.netty.util.internal.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal 
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.958 sec - in 
org.apache.bookkeeper.bookie.LedgerStorageTest
Running org.apache.bookkeeper.bookie.TestSkipListArena
Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.227 sec - in 
org.apache.bookkeeper.bookie.TestSkipListArena
Running org.apache.bookkeeper.bookie.BookieInitializationTest
WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by io.netty.util.internal.ReflectionUtil 
(
 to constructor java.nio.DirectByteBuffer(long,int)
WARNING: Please consider reporting this to the maintainers of 
io.netty.util.internal.ReflectionUtil
WARNING: Use --illegal-access=warn to enable warnings of further illegal 
reflective access operations
WARNING: All illegal access operations will be denied in a future release
Tests run: 20, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 33.248 sec - 
in org.apache.bookkeeper.bookie.BookieInitializationTest
Running org.apache.bookkeeper.server.component.TestServerLifecycleComponent
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.407 sec - in 
org.apache.bookkeeper.server.component.TestServerLifecycleComponent
Running 

Jenkins build is still unstable: bookkeeper_release_branch #16

2018-01-02 Thread Apache Jenkins Server
See 




[GitHub] merlimat commented on a change in pull request #898: Bookies should prioritize recovery reads/writes

2018-01-02 Thread GitBox
merlimat commented on a change in pull request #898: Bookies should prioritize 
recovery reads/writes
URL: https://github.com/apache/bookkeeper/pull/898#discussion_r159200537
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieRequestProcessor.java
 ##
 @@ -152,6 +159,14 @@ public BookieRequestProcessor(ServerConfiguration 
serverCfg, Bookie bookie,
 createExecutor(
 this.serverCfg.getNumLongPollWorkerThreads(),
 "BookieLongPollThread-" + serverCfg.getBookiePort(), 
OrderedScheduler.NO_TASK_LIMIT);
+this.fenceThreadPool = Executors.newCachedThreadPool(
 
 Review comment:
   @sijie This change is a companion of #706. The story here is that in BK 4.6 
if the read rate is very high, maxing out the bookie IO capacity what starts to 
happen is that read requests are piled up in the read executor task queue. 
   
   The read thread is processing the read request, but since they spent a lot 
of time in the queue, the client will have already timed them out and re-sent 
other read requests. 
   
   In #706, we've introduced the max-limit for the size of the executor queue, 
in order to limit the amount of time spent in the queue and rejecting 
additional requests in order to fail-fast. 
   
   The rationale is to avoid to read ops that are going to be timed-out and 
that will not make any progress. 
   
   Another problem with the read operations is that when the timeout/rejections 
happens, it impacts also fencing-reads which are used in recovery. 
   
   This change is to make sure the recovery reads can make progress even though 
there is a very high rate of regular read requests. As Ivan pointed out, the 
recovery reads are critical because they typically prevent a writer from being 
operative, while regular reads are "just" reading data and they can thus be 
deferred to lower priority. 
   
   The original change was doing the fence-read directly in IO thread as the 
way to skip the executor thread queue. I guess that using a dedicated thread 
pool for fence reads is probably a much better option in order not to block the 
IO threads.


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] merlimat closed issue #590: CRC32C for bookkeeper

2018-01-02 Thread GitBox
merlimat closed issue #590: CRC32C for bookkeeper
URL: https://github.com/apache/bookkeeper/issues/590
 
 
   


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] merlimat closed pull request #856: ISSUE #590 (@bug W-4556980@) Reduce excessive CPU usage on client side: add crc3?

2018-01-02 Thread GitBox
merlimat closed pull request #856: ISSUE #590 (@bug W-4556980@) Reduce 
excessive CPU usage on client side: add crc3?
URL: https://github.com/apache/bookkeeper/pull/856
 
 
   

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-proto/src/main/proto/DataFormats.proto 
b/bookkeeper-proto/src/main/proto/DataFormats.proto
index cdade9563..5a5d65f79 100644
--- a/bookkeeper-proto/src/main/proto/DataFormats.proto
+++ b/bookkeeper-proto/src/main/proto/DataFormats.proto
@@ -45,6 +45,7 @@ message LedgerMetadataFormat {
 enum DigestType {
 CRC32 = 1;
 HMAC = 2;
+CRC32C = 3;
 }
 optional DigestType digestType = 7;
 optional bytes password = 8;
diff --git a/bookkeeper-server/pom.xml b/bookkeeper-server/pom.xml
index 04a27d01f..931baf999 100644
--- a/bookkeeper-server/pom.xml
+++ b/bookkeeper-server/pom.xml
@@ -182,6 +182,11 @@
   http-server
   ${project.version}
 
+
+  org.apache.pulsar
+  pulsar-checksum
+  1.20.0-incubating
+
   
   
 
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
index 17df48c84..6dddfb1c3 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java
@@ -636,10 +636,19 @@ boolean isReorderReadSequence() {
  * report fake bytes with a mathching MAC unless it knows the password
  */
 public enum DigestType {
-MAC, CRC32;
+MAC, CRC32, CRC32C;
 
 public static DigestType 
fromApiDigestType(org.apache.bookkeeper.client.api.DigestType digestType) {
-return digestType == 
org.apache.bookkeeper.client.api.DigestType.MAC ? MAC : CRC32;
+switch (digestType) {
+case MAC:
+return DigestType.MAC;
+case CRC32:
+return DigestType.CRC32;
+case CRC32C:
+return DigestType.CRC32C;
+default:
+throw new IllegalArgumentException("Unable to convert 
digest type " + digestType);
+}
 }
 }
 
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java
new file mode 100644
index 0..1923220d7
--- /dev/null
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/CRC32CDigestManager.java
@@ -0,0 +1,65 @@
+package org.apache.bookkeeper.client;
+
+/*
+* 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 com.scurrilous.circe.crc.Sse42Crc32C;
+import io.netty.buffer.ByteBuf;
+import org.apache.commons.lang3.mutable.MutableBoolean;
+import org.apache.commons.lang3.mutable.MutableInt;
+import org.apache.pulsar.checksum.utils.Crc32cChecksum;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class CRC32CDigestManager extends DigestManager {
+static final Logger LOG = 
LoggerFactory.getLogger(CRC32CDigestManager.class);
+
+private final ThreadLocal currentCrc = ThreadLocal
+.withInitial(() -> new MutableInt(0));
+private final ThreadLocal isNewCrc = ThreadLocal
+.withInitial(() -> new MutableBoolean(true));
+
+public CRC32CDigestManager(long ledgerId) {
+super(ledgerId);
+if (!Sse42Crc32C.isSupported()) {
+LOG.error("Sse42Crc32C is not supported, will use less slower 
CRC32C implementation.");
+}
+}
+
+@Override
+int getMacCodeLength() {
+return 4;
+}
+
+@Override
+void populateValueAndReset(ByteBuf buf) {
+buf.writeInt(currentCrc.get().intValue());
+isNewCrc.get().setTrue();
+}
+
+@Override
+void update(ByteBuf data) {
+if (isNewCrc.get().isTrue()) {
+isNewCrc.get().setFalse();
+

Build failed in Jenkins: distributedlog-release-nightly-snapshot #124

2018-01-02 Thread Apache Jenkins Server
See 


--
[...truncated 309.90 KB...]
[INFO] Exclude: GROUPS
[INFO] Exclude: OWNERS
[INFO] Exclude: dist/**/*
[INFO] Exclude: docs/**/*
[INFO] Exclude: scripts/dev/reviewers
[INFO] Exclude: website/**/*
[INFO] Exclude: **/*.md
[INFO] Exclude: **/apidocs/*
[INFO] Exclude: **/dependency-reduced-pom.xml
[INFO] Exclude: **/org/apache/distributedlog/thrift/*
[INFO] Exclude: **/logs/*.log
[INFO] Exclude: **/target/**/*
[INFO] Exclude: .git/**/*
[INFO] Exclude: .github/**/*
[INFO] Exclude: .gitignore
[INFO] Exclude: docker/.gitignore
[INFO] Exclude: .idea/**/*
[INFO] Exclude: **/*.iml
[INFO] Exclude: **/*.iws
[INFO] Exclude: **/*.ipr
[INFO] Exclude: **/.svn/**/*
[INFO] Exclude: .repository/**
[INFO] Exclude: docker/grafana/dashboards/*.json
[INFO] 291 resources included (use -debug for more details)
[INFO] Rat check: Summary over all files. Unapproved: 0, unknown: 0, generated: 
0, approved: 291 licenses.
[INFO] 
[INFO] --- maven-remote-resources-plugin:1.5:process (default) @ 
distributedlog-core ---
[INFO] 
[INFO] --- maven-resources-plugin:2.7:resources (default-resources) @ 
distributedlog-core ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO] Copying 3 resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:compile (default-compile) @ 
distributedlog-core ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 203 source files to 

[WARNING] 
:
 Some input files use or override a deprecated API.
[WARNING] 
:
 Recompile with -Xlint:deprecation for details.
[WARNING] 
:
 Some input files use unchecked or unsafe operations.
[WARNING] 
:
 Recompile with -Xlint:unchecked for details.
[INFO] 
[INFO] --- maven-resources-plugin:2.7:testResources (default-testResources) @ 
distributedlog-core ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] Copying 2 resources
[INFO] Copying 3 resources
[INFO] 
[INFO] --- maven-compiler-plugin:3.1:testCompile (default-testCompile) @ 
distributedlog-core ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 76 source files to 

[WARNING] 
:
 Some input files use or override a deprecated API.
[WARNING] 
:
 Recompile with -Xlint:deprecation for details.
[INFO] 
[INFO] --- maven-checkstyle-plugin:2.17:check (default) @ distributedlog-core 
---
[INFO] Starting audit...
Audit done.
[INFO] 
[INFO] --- maven-surefire-plugin:2.19.1:test (default-test) @ 
distributedlog-core ---
[WARNING] The parameter forkMode is deprecated since version 2.14. Use 
forkCount and reuseForks instead.

---
 T E S T S
---
Running org.apache.distributedlog.TestBKLogWriteHandler
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.58 sec - in 
org.apache.distributedlog.TestBKLogWriteHandler
Running org.apache.distributedlog.bk.TestLedgerAllocator
Tests run: 9, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 26.196 sec - in 
org.apache.distributedlog.bk.TestLedgerAllocator
Running org.apache.distributedlog.bk.TestLedgerAllocatorPool
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 14.709 sec - in 
org.apache.distributedlog.bk.TestLedgerAllocatorPool
Running org.apache.distributedlog.TestEntryPosition
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.195 sec - in 
org.apache.distributedlog.TestEntryPosition
Running org.apache.distributedlog.TestAsyncBulkWrite
Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 32.064 sec - in 
org.apache.distributedlog.TestAsyncBulkWrite
Running org.apache.distributedlog.zk.TestZKWatcherManager
Tests run: 

[GitHub] sijie commented on issue #856: ISSUE #590 (@bug W-4556980@) Reduce excessive CPU usage on client side: add crc3?

2018-01-02 Thread GitBox
sijie commented on issue #856: ISSUE #590 (@bug W-4556980@) Reduce excessive 
CPU usage on client side: add crc3?
URL: https://github.com/apache/bookkeeper/pull/856#issuecomment-354717578
 
 
   @merlimat when you have time, can you review this again? If you are good 
with this, let move forward with 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] sijie closed pull request #929: Issue 922: changes to shading bookkeeper-server jar for branch-4.6

2018-01-02 Thread GitBox
sijie closed pull request #929: Issue 922: changes to shading bookkeeper-server 
jar for branch-4.6
URL: https://github.com/apache/bookkeeper/pull/929
 
 
   

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/pom.xml b/bookkeeper-server/pom.xml
index 4675e0842..7aeb4a1c8 100644
--- a/bookkeeper-server/pom.xml
+++ b/bookkeeper-server/pom.xml
@@ -66,7 +66,7 @@
 
   log4j
   log4j
-   
+   
 
   io.netty
   netty
@@ -204,56 +204,30 @@
 
 
   true
-  
true
+  
false
   true
   shaded
   true
   true
   
 
-  org.apache.bookkeeper:bookkeeper-proto
-  com.google.protobuf:protobuf-java
   com.google.guava:guava
+  com.google.protobuf:protobuf-java
+  org.apache.bookkeeper:bookkeeper-common
+  org.apache.bookkeeper:bookkeeper-proto
+  
org.apache.bookkeeper.stats:bookkeeper-stats-api
 
   
   
-
-  com.google.protobuf
-  
bk-shade.com.google.proto_${protobuf.version}
-
 
   com.google
-  bk-shade.com.google
+  
org.apache.bookkeeper.shaded.com.google
 
   
 
   
 
   
-  
-org.codehaus.mojo
-license-maven-plugin
-1.6
-
-  false
-  ${project.basedir}
-
-
-  
-update-pom-license
-
-  update-file-header
-
-package
-
-  apache_v2
-  
-dependency-reduced-pom.xml
-  
-
-  
-
-  
   
 org.apache.maven.plugins
 maven-jar-plugin
@@ -317,12 +291,6 @@
   ${project.libdir}
   false
 
-
-  ${project.basedir}
-  
-dependency-reduced-pom.xml
-  
-
   

   
diff --git a/pom.xml b/pom.xml
index e173f7123..8a46b0b4f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -58,6 +58,7 @@
 bookkeeper-benchmark
 bookkeeper-stats-providers
 bookkeeper-http
+shaded
 tests
 bookkeeper-dist
   
diff --git a/shaded/bookkeeper-server-shaded/pom.xml 
b/shaded/bookkeeper-server-shaded/pom.xml
new file mode 100644
index 0..f2342aa36
--- /dev/null
+++ b/shaded/bookkeeper-server-shaded/pom.xml
@@ -0,0 +1,125 @@
+
+
+http://maven.apache.org/POM/4.0.0 
http://maven.apache.org/xsd/maven-4.0.0.xsd; 
xmlns="http://maven.apache.org/POM/4.0.0;
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;>
+  4.0.0
+  
+org.apache.bookkeeper
+shaded-parent
+4.6.0-SNAPSHOT
+..
+  
+  bookkeeper-server-shaded
+  Apache BookKeeper :: Shaded :: bookkeeper-server-shaded
+  
+UTF-8
+  
+  
+
+  org.apache.bookkeeper
+  bookkeeper-server
+  ${project.version}
+  
+
+  org.slf4j
+  slf4j-log4j12
+
+
+  log4j
+  log4j
+
+  
+
+  
+  
+
+  
+org.apache.maven.plugins
+maven-shade-plugin
+${maven-shade-plugin.version}
+
+  
+package
+
+  shade
+
+
+  true
+  
true
+  false
+  
+
+  com.google.guava:guava
+  com.google.protobuf:protobuf-java
+  org.apache.bookkeeper:bookkeeper-common
+  org.apache.bookkeeper:bookkeeper-proto
+  org.apache.bookkeeper:bookkeeper-server
+  
org.apache.bookkeeper.stats:bookkeeper-stats-api
+
+  
+  
+
+  com.google
+  
org.apache.bookkeeper.shaded.com.google
+
+  
+
+  
+
+  
+  
+org.codehaus.mojo
+license-maven-plugin
+1.6
+
+  false
+  ${project.basedir}
+
+
+  
+update-pom-license
+
+  update-file-header
+
+package
+
+  apache_v2
+  
+dependency-reduced-pom.xml
+  
+
+  
+
+  
+  
+maven-clean-plugin
+2.5