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

2018-01-07 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_r160072577
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
 ##
 @@ -1128,122 +1118,40 @@ public void processResult(int rc, String s, Object 
ctx) {
  *removed without prompt.
  * @return Returns true if format succeeds else false.
  */
-public static boolean format(ClientConfiguration conf,
+public static boolean format(ServerConfiguration 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 = 
RegistrationManager.instantiateRegistrationManager(conf)) {
+boolean ledgerRootExists = rm.prepareFormat(conf);
 
 // If old data was there then confirm with admin.
+boolean doFormat = true;
 if (ledgerRootExists) {
-boolean confirm = false;
 if (!isInteractive) {
-// If non interactive and force is set, then delete old
-// data.
+// If non interactive and force is set, then delete old 
data.
 if (force) {
-confirm = true;
+doFormat = true;
 } else {
-confirm = false;
+doFormat = false;
 }
 } else {
 // Confirm with the admin.
-confirm = IOUtils
-.confirmPrompt("Ledger root already exists. "
-+ "Are you sure to format bookkeeper 
metadata? "
-+ "This may cause data loss.");
-}
-if (!confirm) {
-LOG.error("BookKeeper metadata Format aborted!!");
-return false;
-}
-}
-bkc = new BookKeeper(conf, zkc);
-// Format all ledger metadata layout
-bkc.ledgerManagerFactory.format(conf, zkc);
-
-// Clear underreplicated ledgers
-try {
-ZKUtil.deleteRecursive(zkc, 
ZkLedgerUnderreplicationManager.getBasePath(conf.getZkLedgersRootPath())
-+ BookKeeperConstants.DEFAULT_ZK_LEDGERS_ROOT_PATH);
-} catch (KeeperException.NoNodeException e) {
-if (LOG.isDebugEnabled()) {
-LOG.debug("underreplicated ledgers root path node not 
exists in zookeeper to delete");
-}
-}
-
-// Clear underreplicatedledger locks
-try {
-ZKUtil.deleteRecursive(zkc, 
ZkLedgerUnderreplicationManager.getBasePath(conf.getZkLedgersRootPath())
-+ '/' + BookKeeperConstants.UNDER_REPLICATION_LOCK);
-} catch (KeeperException.NoNodeException e) {
-if (LOG.isDebugEnabled()) {
-LOG.debug("underreplicatedledger locks node not exists in 
zookeeper to delete");
-}
-}
-
-// Clear the cookies
-try {
-ZKUtil.deleteRecursive(zkc, conf.getZkLedgersRootPath()
-+ "/cookies");
-} catch (KeeperException.NoNodeException e) {
-if (LOG.isDebugEnabled()) {
-LOG.debug("cookies node not exists in zookeeper to 
delete");
+

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

2018-01-07 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_r160072057
 
 

 ##
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
 ##
 @@ -1512,13 +1470,10 @@ int runCmd(CommandLine cmdLine) throws Exception {
 printUsage();
 return 1;
 }
-ZooKeeper zk = null;
-try {
-zk = ZooKeeperClient.newBuilder()
-.connectString(bkConf.getZkServers())
-.sessionTimeoutMs(bkConf.getZkTimeout())
-.build();
-LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(bkConf, zk);
+
+try (LedgerManagerFactory mFactory = 
LedgerManagerFactory.newLedgerManagerFactory(
+bkConf,
+
RegistrationManager.instantiateRegistrationManager(bkConf).getLayoutManager())) 
{
 
 Review comment:
   `RegistrationManager` will never be released. You need to do:
   
   ```
   try (RegistrationManager rm = ...) {
   try (LedgerManagerFactory ...) {
   }
   }
   ```


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