You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2018/01/03 19:38:27 UTC
[GitHub] reddycharan commented on a change in pull request #936: Issue 897: un-bind zookeeper from bookkeeper admin
reddycharan 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_r159509519
##########
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<ACL> 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");
+ doFormat = IOUtils
+ .confirmPrompt("Ledger root already exists. "
+ + "Are you sure to format bookkeeper metadata? "
+ + "This may cause data loss.");
}
}
- // Clear the INSTANCEID
- try {
- zkc.delete(conf.getZkLedgersRootPath() + "/"
- + BookKeeperConstants.INSTANCEID, -1);
- } catch (KeeperException.NoNodeException e) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("INSTANCEID not exists in zookeeper to delete");
- }
+ if (!doFormat) {
+ return false;
}
- // create INSTANCEID
- String instanceId = UUID.randomUUID().toString();
- zkc.create(conf.getZkLedgersRootPath() + "/"
- + BookKeeperConstants.INSTANCEID, instanceId.getBytes(UTF_8),
- zkAcls, CreateMode.PERSISTENT);
+ BookKeeper bkc = new BookKeeper(new ClientConfiguration(conf));
+ bkc.ledgerManagerFactory.format(conf, bkc.regClient.getLayoutManager());
- LOG.info("Successfully formatted BookKeeper metadata");
- } finally {
- if (null != bkc) {
- bkc.close();
- }
- if (null != zkc) {
- zkc.close();
- }
+ return rm.format(conf);
Review comment:
it doesn't look appropriate to make 3 different calls here
rm.prepareFormat(conf)
bkc.ledgerManagerFactory.format(conf, bkc.regClient.getLayoutManager())
rm.format(conf)
is it not possible to abstract it out to single rm method call, probably with arguments?
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services