sijie closed pull request #1373: Make bookie state manager & registration
manager reusable outside of bookie
URL: https://github.com/apache/bookkeeper/pull/1373
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java
index de2c797a8..370e06d82 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java
@@ -24,15 +24,20 @@
import static org.apache.bookkeeper.bookie.BookKeeperServerStats.SERVER_STATUS;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
+import java.io.File;
import java.io.IOException;
+import java.io.UncheckedIOException;
import java.net.UnknownHostException;
+import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.discover.RegistrationManager;
import org.apache.bookkeeper.meta.MetadataBookieDriver;
import org.apache.bookkeeper.stats.Gauge;
import org.apache.bookkeeper.stats.NullStatsLogger;
@@ -48,8 +53,7 @@
public class BookieStateManager implements StateManager {
private static final Logger LOG =
LoggerFactory.getLogger(BookieStateManager.class);
private final ServerConfiguration conf;
- private final LedgerDirsManager ledgerDirsManager;
-
+ private final List<File> statusDirs;
// use an executor to execute the state changes task
final ExecutorService stateService = Executors.newSingleThreadExecutor(
@@ -67,18 +71,38 @@
private final String bookieId;
private ShutdownHandler shutdownHandler;
- private final MetadataBookieDriver metadataDriver;
+ private final Supplier<RegistrationManager> rm;
// Expose Stats
private final StatsLogger statsLogger;
- public BookieStateManager(ServerConfiguration conf, StatsLogger
statsLogger,
- MetadataBookieDriver metadataDriver, LedgerDirsManager
ledgerDirsManager) throws IOException {
+ public BookieStateManager(ServerConfiguration conf,
+ StatsLogger statsLogger,
+ MetadataBookieDriver metadataDriver,
+ LedgerDirsManager ledgerDirsManager) throws
IOException {
+ this(
+ conf,
+ statsLogger,
+ () -> null == metadataDriver ? null :
metadataDriver.getRegistrationManager(),
+ ledgerDirsManager.getAllLedgerDirs(),
+ () -> {
+ try {
+ return Bookie.getBookieAddress(conf).toString();
+ } catch (UnknownHostException e) {
+ throw new UncheckedIOException("Failed to resolve bookie
id", e);
+ }
+ });
+ }
+ public BookieStateManager(ServerConfiguration conf,
+ StatsLogger statsLogger,
+ Supplier<RegistrationManager> rm,
+ List<File> statusDirs,
+ Supplier<String> bookieIdSupplier) throws
IOException {
this.conf = conf;
this.statsLogger = statsLogger;
- this.metadataDriver = metadataDriver;
- this.ledgerDirsManager = ledgerDirsManager;
+ this.rm = rm;
+ this.statusDirs = statusDirs;
// ZK ephemeral node for this Bookie.
- this.bookieId = getMyId();
+ this.bookieId = bookieIdSupplier.get();
// 1 : up, 0 : readonly, -1 : unregistered
statsLogger.registerGauge(SERVER_STATUS, new Gauge<Number>() {
@Override
@@ -99,6 +123,10 @@ public Number getSample() {
});
}
+ private boolean isRegistrationManagerDisabled() {
+ return null == rm || null == rm.get();
+ }
+
@VisibleForTesting
BookieStateManager(ServerConfiguration conf, MetadataBookieDriver
metadataDriver) throws IOException {
this(conf, NullStatsLogger.INSTANCE, metadataDriver, new
LedgerDirsManager(conf, conf.getLedgerDirs(),
@@ -111,7 +139,7 @@ public void initState(){
if (forceReadOnly.get()) {
this.bookieStatus.setToReadOnlyMode();
} else if (conf.isPersistBookieStatusEnabled()) {
-
this.bookieStatus.readFromDirectories(ledgerDirsManager.getAllLedgerDirs());
+ this.bookieStatus.readFromDirectories(statusDirs);
}
running = true;
}
@@ -215,7 +243,7 @@ void doRegisterBookie() throws IOException {
}
private void doRegisterBookie(boolean isReadOnly) throws IOException {
- if (null == metadataDriver) {
+ if (isRegistrationManagerDisabled()) {
// registration manager is null, means not register itself to
metadata store.
LOG.info("null registration manager while do register");
return;
@@ -223,7 +251,7 @@ private void doRegisterBookie(boolean isReadOnly) throws
IOException {
rmRegistered.set(false);
try {
- metadataDriver.getRegistrationManager().registerBookie(bookieId,
isReadOnly);
+ rm.get().registerBookie(bookieId, isReadOnly);
rmRegistered.set(true);
} catch (BookieException e) {
throw new IOException(e);
@@ -242,10 +270,10 @@ public void doTransitionToWritableMode() {
}
LOG.info("Transitioning Bookie to Writable mode and will serve
read/write requests.");
if (conf.isPersistBookieStatusEnabled()) {
-
bookieStatus.writeToDirectories(ledgerDirsManager.getAllLedgerDirs());
+ bookieStatus.writeToDirectories(statusDirs);
}
// change zookeeper state only when using zookeeper
- if (null == metadataDriver) {
+ if (isRegistrationManagerDisabled()) {
return;
}
try {
@@ -257,7 +285,7 @@ public void doTransitionToWritableMode() {
}
// clear the readonly state
try {
- metadataDriver.getRegistrationManager().unregisterBookie(bookieId,
true);
+ rm.get().unregisterBookie(bookieId, true);
} catch (BookieException e) {
// if we failed when deleting the readonly flag in zookeeper, it
is OK since client would
// already see the bookie in writable list. so just log the
exception
@@ -286,14 +314,14 @@ public void doTransitionToReadOnlyMode() {
+ " and will serve only read requests from clients!");
// persist the bookie status if we enable this
if (conf.isPersistBookieStatusEnabled()) {
-
this.bookieStatus.writeToDirectories(ledgerDirsManager.getAllLedgerDirs());
+ this.bookieStatus.writeToDirectories(statusDirs);
}
// change zookeeper state only when using zookeeper
- if (null == metadataDriver) {
+ if (isRegistrationManagerDisabled()) {
return;
}
try {
- metadataDriver.getRegistrationManager().registerBookie(bookieId,
true);
+ rm.get().registerBookie(bookieId, true);
} catch (BookieException e) {
LOG.error("Error in transition to ReadOnly Mode."
+ " Shutting down", e);
@@ -305,10 +333,6 @@ public void setShutdownHandler(ShutdownHandler handler){
shutdownHandler = handler;
}
- private String getMyId() throws UnknownHostException {
- return Bookie.getBookieAddress(conf).toString();
- }
-
@VisibleForTesting
public ShutdownHandler getShutdownHandler(){
return shutdownHandler;
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationManager.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationManager.java
index 15b03d560..eab9e4f18 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationManager.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/discover/ZKRegistrationManager.java
@@ -108,11 +108,17 @@
public ZKRegistrationManager(ServerConfiguration conf,
ZooKeeper zk,
RegistrationListener listener) {
+ this(conf, zk, ZKMetadataDriverBase.resolveZkLedgersRootPath(conf),
listener);
+ }
+
+ public ZKRegistrationManager(ServerConfiguration conf,
+ ZooKeeper zk,
+ String ledgersRootPath,
+ RegistrationListener listener) {
this.conf = conf;
this.zk = zk;
this.zkAcls = ZkUtils.getACLs(conf);
-
- this.ledgersRootPath =
ZKMetadataDriverBase.resolveZkLedgersRootPath(conf);
+ this.ledgersRootPath = ledgersRootPath;
this.cookiePath = ledgersRootPath + "/" + COOKIE_NODE;
this.bookieRegistrationPath = ledgersRootPath + "/" + AVAILABLE_NODE;
this.bookieReadonlyRegistrationPath = this.bookieRegistrationPath +
"/" + READONLY;
diff --git
a/tests/backward-compat/upgrade/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatUpgrade.groovy
b/tests/backward-compat/upgrade/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatUpgrade.groovy
index 8e0d8d71f..940501df6 100644
---
a/tests/backward-compat/upgrade/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatUpgrade.groovy
+++
b/tests/backward-compat/upgrade/src/test/groovy/org/apache/bookkeeper/tests/backwardcompat/TestCompatUpgrade.groovy
@@ -178,12 +178,12 @@ class TestCompatUpgrade {
@Test
public void test460to461() throws Exception {
- testUpgrade("4.6.0", "4.6.1")
+ testUpgrade("4.6.0", "4.6.1", false, true)
}
@Test
public void test461to462() throws Exception {
- testUpgrade("4.6.1", "4.6.2")
+ testUpgrade("4.6.1", "4.6.2", false, true)
}
@Test
@@ -193,6 +193,6 @@ class TestCompatUpgrade {
@Test
public void test470toCurrentMaster() throws Exception {
- testUpgrade("4.7.0", System.getProperty("currentVersion"), false, true)
+ testUpgrade("4.7.0", System.getProperty("currentVersion"))
}
}
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
