You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by si...@apache.org on 2018/03/17 04:31:30 UTC
[bookkeeper] branch master updated: Improve
`BookKeeperAdmin#format` to avoid constructing redundant resources
This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
The following commit(s) were added to refs/heads/master by this push:
new b922513 Improve `BookKeeperAdmin#format` to avoid constructing redundant resources
b922513 is described below
commit b92251379b1183fea3fe124637390ddc72d5a069
Author: Sijie Guo <si...@apache.org>
AuthorDate: Fri Mar 16 21:31:24 2018 -0700
Improve `BookKeeperAdmin#format` to avoid constructing redundant resources
Descriptions of the changes in this PR:
*Problem*
After metadata service uri change, all the metadata managers can be retrieved via metadata drivers.
So there is no need to construct another bookkeeper client to get ledger manager factory for formatting ledger metadata.
*Solution*
- remove constructing bookkeeper client and use the metadata driver to get ledger manager factory to format ledger metadata
- expose layout manager in bookie driver as well
- since we are adding `getLayoutManager` in bookie driver, we need to update `MetadataDriversTest`. cleanup the test drivers in `MetadataDriversTest`.
Related Issue: #1269
Author: Sijie Guo <si...@apache.org>
Reviewers: Enrico Olivelli <eo...@gmail.com>, Jia Zhai <None>
This closes #1270 from sijie/improve_format
---
.../apache/bookkeeper/client/BookKeeperAdmin.java | 13 ++--
.../bookkeeper/meta/MetadataBookieDriver.java | 7 ++
.../bookkeeper/meta/MetadataDriversTest.java | 75 ++++++----------------
3 files changed, 35 insertions(+), 60 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
index 7671721..56865db 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
@@ -21,6 +21,7 @@
package org.apache.bookkeeper.client;
import static com.google.common.base.Preconditions.checkArgument;
+import static org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithMetadataBookieDriver;
import static org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithRegistrationManager;
import com.google.common.collect.Lists;
@@ -1133,9 +1134,9 @@ public class BookKeeperAdmin implements AutoCloseable {
*/
public static boolean format(ServerConfiguration conf,
boolean isInteractive, boolean force) throws Exception {
- return runFunctionWithRegistrationManager(conf, rm -> {
+ return runFunctionWithMetadataBookieDriver(conf, driver -> {
try {
- boolean ledgerRootExists = rm.prepareFormat();
+ boolean ledgerRootExists = driver.getRegistrationManager().prepareFormat();
// If old data was there then confirm with admin.
boolean doFormat = true;
@@ -1160,11 +1161,11 @@ public class BookKeeperAdmin implements AutoCloseable {
return false;
}
- try (BookKeeper bkc = new BookKeeper(new ClientConfiguration(conf))) {
- bkc.ledgerManagerFactory.format(conf, bkc.getMetadataClientDriver().getLayoutManager());
- }
+ driver.getLedgerManagerFactory().format(
+ conf,
+ driver.getLayoutManager());
- return rm.format();
+ return driver.getRegistrationManager().format();
} catch (Exception e) {
throw new UncheckedExecutionException(e.getMessage(), e);
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MetadataBookieDriver.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MetadataBookieDriver.java
index 9b11f28..b4d0e0d 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MetadataBookieDriver.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/MetadataBookieDriver.java
@@ -64,6 +64,13 @@ public interface MetadataBookieDriver extends AutoCloseable {
LedgerManagerFactory getLedgerManagerFactory()
throws MetadataException;
+ /**
+ * Return the layout manager.
+ *
+ * @return the layout manager.
+ */
+ LayoutManager getLayoutManager();
+
@Override
void close();
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MetadataDriversTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MetadataDriversTest.java
index b586890..2d69cd2 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MetadataDriversTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/meta/MetadataDriversTest.java
@@ -51,7 +51,7 @@ import org.junit.Test;
*/
public class MetadataDriversTest {
- static class ClientDriver1 implements MetadataClientDriver {
+ abstract static class TestClientDriver implements MetadataClientDriver {
@Override
public MetadataClientDriver initialize(ClientConfiguration conf,
@@ -62,11 +62,6 @@ public class MetadataDriversTest {
}
@Override
- public String getScheme() {
- return "driver1";
- }
-
- @Override
public RegistrationClient getRegistrationClient() {
return mock(RegistrationClient.class);
}
@@ -86,43 +81,25 @@ public class MetadataDriversTest {
}
}
- static class ClientDriver2 implements MetadataClientDriver {
-
- @Override
- public MetadataClientDriver initialize(ClientConfiguration conf,
- ScheduledExecutorService scheduler,
- StatsLogger statsLogger,
- Optional<Object> ctx) throws MetadataException {
- return this;
- }
+ static class ClientDriver1 extends TestClientDriver {
@Override
public String getScheme() {
- return "driver2";
+ return "driver1";
}
- @Override
- public RegistrationClient getRegistrationClient() {
- return mock(RegistrationClient.class);
- }
+ }
- @Override
- public LedgerManagerFactory getLedgerManagerFactory() throws MetadataException {
- return mock(LedgerManagerFactory.class);
- }
+ static class ClientDriver2 extends TestClientDriver {
@Override
- public LayoutManager getLayoutManager() {
- return mock(LayoutManager.class);
+ public String getScheme() {
+ return "driver2";
}
- @Override
- public void close() {
- }
}
- static class BookieDriver1 implements MetadataBookieDriver {
-
+ abstract static class TestBookieDriver implements MetadataBookieDriver {
@Override
public MetadataBookieDriver initialize(ServerConfiguration conf,
RegistrationListener listener,
@@ -131,11 +108,6 @@ public class MetadataDriversTest {
}
@Override
- public String getScheme() {
- return "driver1";
- }
-
- @Override
public RegistrationManager getRegistrationManager() {
return mock(RegistrationManager.class);
}
@@ -146,37 +118,32 @@ public class MetadataDriversTest {
}
@Override
- public void close() {
+ public LayoutManager getLayoutManager() {
+ return mock(LayoutManager.class);
}
- }
-
- static class BookieDriver2 implements MetadataBookieDriver {
@Override
- public MetadataBookieDriver initialize(ServerConfiguration conf,
- RegistrationListener listener,
- StatsLogger statsLogger) throws MetadataException {
- return this;
+ public void close() {
+
}
+ }
+
+ static class BookieDriver1 extends TestBookieDriver {
@Override
public String getScheme() {
- return "driver2";
+ return "driver1";
}
- @Override
- public RegistrationManager getRegistrationManager() {
- return mock(RegistrationManager.class);
- }
+ }
- @Override
- public LedgerManagerFactory getLedgerManagerFactory() throws MetadataException {
- return mock(LedgerManagerFactory.class);
- }
+ static class BookieDriver2 extends TestBookieDriver {
@Override
- public void close() {
+ public String getScheme() {
+ return "driver2";
}
+
}
private Map<String, MetadataClientDriverInfo> savedClientDrivers;
--
To stop receiving notification emails like this one, please contact
sijie@apache.org.