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 2017/11/30 02:53:51 UTC
[GitHub] sijie closed pull request #712: Issue 544: Bootup cookie validation considers an empty journal to signify a new bookie
sijie closed pull request #712: Issue 544: Bootup cookie validation considers an empty journal to signify a new bookie
URL: https://github.com/apache/bookkeeper/pull/712
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/Bookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
index 83888c173..09833959a 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
@@ -35,7 +35,6 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
import com.google.common.util.concurrent.SettableFuture;
import com.google.common.util.concurrent.ThreadFactoryBuilder;
import io.netty.buffer.ByteBuf;
@@ -49,8 +48,8 @@
import java.nio.ByteBuffer;
import java.nio.file.FileStore;
import java.nio.file.Files;
+import java.util.Arrays;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -68,7 +67,9 @@
import org.apache.bookkeeper.bookie.BookieException.BookieIllegalOpException;
import org.apache.bookkeeper.bookie.BookieException.CookieNotFoundException;
import org.apache.bookkeeper.bookie.BookieException.DiskPartitionDuplicationException;
+import org.apache.bookkeeper.bookie.BookieException.InvalidCookieException;
import org.apache.bookkeeper.bookie.BookieException.MetadataStoreException;
+import org.apache.bookkeeper.bookie.BookieException.UnknownBookieIdException;
import org.apache.bookkeeper.bookie.Journal.JournalScanner;
import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener;
import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException;
@@ -96,6 +97,7 @@
import org.apache.commons.configuration.ConfigurationException;
import org.apache.commons.io.FileUtils;
import org.apache.commons.lang3.mutable.MutableBoolean;
+import org.apache.commons.lang3.tuple.Pair;
import org.apache.zookeeper.KeeperException;
import org.apache.zookeeper.ZooKeeper;
import org.slf4j.Logger;
@@ -277,122 +279,10 @@ private void checkEnvironment(RegistrationManager rm) throws BookieException, IO
return;
}
- if (conf.getAllowStorageExpansion()) {
- checkEnvironmentWithStorageExpansion(conf, rm, journalDirectories, allLedgerDirs);
- return;
- }
-
- try {
- boolean newEnv = false;
- List<File> missedCookieDirs = new ArrayList<File>();
- List<Cookie> journalCookies = Lists.newArrayList();
- // try to read cookie from journal directory.
- for (File journalDirectory : journalDirectories) {
- try {
- Cookie journalCookie = Cookie.readFromDirectory(journalDirectory);
- journalCookies.add(journalCookie);
- if (journalCookie.isBookieHostCreatedFromIp()) {
- conf.setUseHostNameAsBookieID(false);
- } else {
- conf.setUseHostNameAsBookieID(true);
- }
- } catch (FileNotFoundException fnf) {
- newEnv = true;
- missedCookieDirs.add(journalDirectory);
- }
- }
-
- String instanceId = rm.getClusterInstanceId();
- Cookie.Builder builder = Cookie.generateCookie(conf);
- if (null != instanceId) {
- builder.setInstanceId(instanceId);
- }
- Cookie masterCookie = builder.build();
- Versioned<Cookie> rmCookie = null;
- try {
- rmCookie = Cookie.readFromRegistrationManager(rm, conf);
- // If allowStorageExpansion option is set, we should
- // make sure that the new set of ledger/index dirs
- // is a super set of the old; else, we fail the cookie check
- masterCookie.verifyIsSuperSet(rmCookie.getValue());
- } catch (CookieNotFoundException e) {
- // can occur in cases:
- // 1) new environment or
- // 2) done only metadata format and started bookie server.
- }
- for (File journalDirectory : journalDirectories) {
- checkDirectoryStructure(journalDirectory);
- }
- if (!newEnv) {
- for (Cookie journalCookie: journalCookies) {
- masterCookie.verify(journalCookie);
- }
- }
-
-
- for (File dir : allLedgerDirs) {
- checkDirectoryStructure(dir);
- try {
- Cookie c = Cookie.readFromDirectory(dir);
- masterCookie.verify(c);
- } catch (FileNotFoundException fnf) {
- missedCookieDirs.add(dir);
- }
- }
-
- if (!newEnv && missedCookieDirs.size() > 0) {
- // If we find that any of the dirs in missedCookieDirs, existed
- // previously, we stop because we could be missing data
- // Also, if a new ledger dir is being added, we make sure that
- // that dir is empty. Else, we reject the request
- Set<String> existingLedgerDirs = Sets.newHashSet();
- for (Cookie journalCookie : journalCookies) {
- Collections.addAll(existingLedgerDirs, journalCookie.getLedgerDirPathsFromCookie());
- }
- List<File> dirsMissingData = new ArrayList<File>();
- List<File> nonEmptyDirs = new ArrayList<File>();
- for (File dir : missedCookieDirs) {
- if (existingLedgerDirs.contains(dir.getParent())) {
- // if one of the existing ledger dirs doesn't have cookie,
- // let us not proceed further
- dirsMissingData.add(dir);
- continue;
- }
- String[] content = dir.list();
- if (content != null && content.length != 0) {
- nonEmptyDirs.add(dir);
- }
- }
- if (dirsMissingData.size() > 0 || nonEmptyDirs.size() > 0) {
- LOG.error("Either not all local directories have cookies or directories being added "
- + " newly are not empty. "
- + "Directories missing cookie file are: " + dirsMissingData
- + " New directories that are not empty are: " + nonEmptyDirs);
- throw new BookieException.InvalidCookieException();
- }
- }
-
- if (missedCookieDirs.size() > 0) {
- LOG.info("Stamping new cookies on all dirs {}", missedCookieDirs);
- for (File journalDirectory : journalDirectories) {
- masterCookie.writeToDirectory(journalDirectory);
- }
- for (File dir : allLedgerDirs) {
- masterCookie.writeToDirectory(dir);
- }
- masterCookie.writeToRegistrationManager(rm, conf, rmCookie != null ? rmCookie.getVersion() : Version.NEW);
- }
-
- List<File> ledgerDirs = ledgerDirsManager.getAllLedgerDirs();
- checkIfDirsOnSameDiskPartition(ledgerDirs);
- List<File> indexDirs = indexDirsManager.getAllLedgerDirs();
- checkIfDirsOnSameDiskPartition(indexDirs);
- checkIfDirsOnSameDiskPartition(journalDirectories);
+ checkEnvironmentWithStorageExpansion(conf, rm, journalDirectories, allLedgerDirs);
- } catch (IOException ioe) {
- LOG.error("Error accessing cookie on disks", ioe);
- throw new BookieException.InvalidCookieException(ioe);
- }
+ checkIfDirsOnSameDiskPartition(allLedgerDirs);
+ checkIfDirsOnSameDiskPartition(journalDirectories);
}
/**
@@ -441,110 +331,164 @@ private void checkIfDirsOnSameDiskPartition(List<File> dirs) throws DiskPartitio
}
}
- public static void checkEnvironmentWithStorageExpansion(
- ServerConfiguration conf,
- RegistrationManager rm,
- List<File> journalDirectories,
- List<File> allLedgerDirs) throws BookieException {
+ static List<BookieSocketAddress> possibleBookieIds(ServerConfiguration conf)
+ throws BookieException {
+ // we need to loop through all possible bookie identifiers to ensure it is treated as a new environment
+ // just because of bad configuration
+ List<BookieSocketAddress> addresses = Lists.newArrayListWithExpectedSize(3);
try {
- boolean newEnv = false;
- List<File> missedCookieDirs = new ArrayList<File>();
- List<Cookie> journalCookies = Lists.newArrayList();
- // try to read cookie from journal directory.
- for (File journalDirectory : journalDirectories) {
- try {
- Cookie journalCookie = Cookie.readFromDirectory(journalDirectory);
- journalCookies.add(journalCookie);
- if (journalCookie.isBookieHostCreatedFromIp()) {
- conf.setUseHostNameAsBookieID(false);
- } else {
- conf.setUseHostNameAsBookieID(true);
- }
- } catch (FileNotFoundException fnf) {
- newEnv = true;
- missedCookieDirs.add(journalDirectory);
- }
+ // ip address
+ addresses.add(getBookieAddress(
+ new ServerConfiguration(conf).setUseHostNameAsBookieID(false).setAdvertisedAddress(null)));
+ // host name
+ addresses.add(getBookieAddress(
+ new ServerConfiguration(conf).setUseHostNameAsBookieID(true).setAdvertisedAddress(null)));
+ // advertised address
+ if (null != conf.getAdvertisedAddress()) {
+ addresses.add(getBookieAddress(conf));
}
+ } catch (UnknownHostException e) {
+ throw new UnknownBookieIdException(e);
+ }
+ return addresses;
+ }
- String instanceId = rm.getClusterInstanceId();
- Cookie.Builder builder = Cookie.generateCookie(conf);
- if (null != instanceId) {
- builder.setInstanceId(instanceId);
- }
- Cookie masterCookie = builder.build();
- Versioned<Cookie> rmCookie = null;
+ static Versioned<Cookie> readAndVerifyCookieFromRegistrationManager(
+ Cookie masterCookie, RegistrationManager rm,
+ List<BookieSocketAddress> addresses, boolean allowExpansion)
+ throws BookieException {
+ Versioned<Cookie> rmCookie = null;
+ for (BookieSocketAddress address : addresses) {
try {
- rmCookie = Cookie.readFromRegistrationManager(rm, conf);
+ rmCookie = Cookie.readFromRegistrationManager(rm, address);
// If allowStorageExpansion option is set, we should
// make sure that the new set of ledger/index dirs
// is a super set of the old; else, we fail the cookie check
- masterCookie.verifyIsSuperSet(rmCookie.getValue());
- } catch (CookieNotFoundException e) {
- // can occur in cases:
- // 1) new environment or
- // 2) done only metadata format and started bookie server.
- }
- for (File journalDirectory : journalDirectories) {
- checkDirectoryStructure(journalDirectory);
- }
- if (!newEnv) {
- for (Cookie journalCookie: journalCookies) {
- masterCookie.verifyIsSuperSet(journalCookie);
+ if (allowExpansion) {
+ masterCookie.verifyIsSuperSet(rmCookie.getValue());
+ } else {
+ masterCookie.verify(rmCookie.getValue());
}
+ } catch (CookieNotFoundException e) {
+ continue;
}
+ }
+ return rmCookie;
+ }
-
- for (File dir : allLedgerDirs) {
- checkDirectoryStructure(dir);
- try {
- Cookie c = Cookie.readFromDirectory(dir);
+ private static Pair<List<File>, List<Cookie>> verifyAndGetMissingDirs(
+ Cookie masterCookie, boolean allowExpansion, List<File> dirs)
+ throws InvalidCookieException, IOException {
+ List<File> missingDirs = Lists.newArrayList();
+ List<Cookie> existedCookies = Lists.newArrayList();
+ for (File dir : dirs) {
+ checkDirectoryStructure(dir);
+ try {
+ Cookie c = Cookie.readFromDirectory(dir);
+ if (allowExpansion) {
masterCookie.verifyIsSuperSet(c);
- } catch (FileNotFoundException fnf) {
- missedCookieDirs.add(dir);
+ } else {
+ masterCookie.verify(c);
}
+ existedCookies.add(c);
+ } catch (FileNotFoundException fnf) {
+ missingDirs.add(dir);
}
+ }
+ return Pair.of(missingDirs, existedCookies);
+ }
- if (!newEnv && missedCookieDirs.size() > 0) {
- // If we find that any of the dirs in missedCookieDirs, existed
- // previously, we stop because we could be missing data
- // Also, if a new ledger dir is being added, we make sure that
- // that dir is empty. Else, we reject the request
- Set<String> existingLedgerDirs = Sets.newHashSet();
- for (Cookie journalCookie : journalCookies) {
- Collections.addAll(existingLedgerDirs, journalCookie.getLedgerDirPathsFromCookie());
- }
- List<File> dirsMissingData = new ArrayList<File>();
- List<File> nonEmptyDirs = new ArrayList<File>();
- for (File dir : missedCookieDirs) {
- if (existingLedgerDirs.contains(dir.getParent())) {
- // if one of the existing ledger dirs doesn't have cookie,
- // let us not proceed further
- dirsMissingData.add(dir);
- continue;
- }
- String[] content = dir.list();
- if (content != null && content.length != 0) {
- nonEmptyDirs.add(dir);
- }
- }
- if (dirsMissingData.size() > 0 || nonEmptyDirs.size() > 0) {
- LOG.error("Either not all local directories have cookies or directories being added "
- + " newly are not empty. "
- + "Directories missing cookie file are: " + dirsMissingData
- + " New directories that are not empty are: " + nonEmptyDirs);
- throw new BookieException.InvalidCookieException();
- }
+ private static void stampNewCookie(ServerConfiguration conf,
+ Cookie masterCookie,
+ RegistrationManager rm,
+ Version version,
+ List<File> journalDirectories,
+ List<File> allLedgerDirs)
+ throws BookieException, IOException {
+ // backfill all the directories that miss cookies (for storage expansion)
+ LOG.info("Stamping new cookies on all dirs {} {}",
+ journalDirectories, allLedgerDirs);
+ for (File journalDirectory : journalDirectories) {
+ masterCookie.writeToDirectory(journalDirectory);
+ }
+ for (File dir : allLedgerDirs) {
+ masterCookie.writeToDirectory(dir);
+ }
+ masterCookie.writeToRegistrationManager(rm, conf, version);
+ }
+
+ public static void checkEnvironmentWithStorageExpansion(
+ ServerConfiguration conf,
+ RegistrationManager rm,
+ List<File> journalDirectories,
+ List<File> allLedgerDirs) throws BookieException {
+ try {
+ // 1. retrieve the instance id
+ String instanceId = rm.getClusterInstanceId();
+
+ // 2. build the master cookie from the configuration
+ Cookie.Builder builder = Cookie.generateCookie(conf);
+ if (null != instanceId) {
+ builder.setInstanceId(instanceId);
+ }
+ Cookie masterCookie = builder.build();
+ boolean allowExpansion = conf.getAllowStorageExpansion();
+
+ // 3. read the cookie from registration manager. it is the `source-of-truth` of a given bookie.
+ // if it doesn't exist in registration manager, this bookie is a new bookie, otherwise it is
+ // an old bookie.
+ List<BookieSocketAddress> possibleBookieIds = possibleBookieIds(conf);
+ final Versioned<Cookie> rmCookie
+ = readAndVerifyCookieFromRegistrationManager(
+ masterCookie, rm, possibleBookieIds, allowExpansion);
+
+ // 4. check if the cookie appear in all the directories.
+ List<File> missedCookieDirs = new ArrayList<>();
+ List<Cookie> existingCookies = Lists.newArrayList();
+ if (null != rmCookie) {
+ existingCookies.add(rmCookie.getValue());
}
- if (missedCookieDirs.size() > 0) {
- LOG.info("Stamping new cookies on all dirs {}", missedCookieDirs);
- for (File journalDirectory : journalDirectories) {
- masterCookie.writeToDirectory(journalDirectory);
- }
- for (File dir : allLedgerDirs) {
- masterCookie.writeToDirectory(dir);
+ // 4.1 verify the cookies in journal directories
+ Pair<List<File>, List<Cookie>> journalResult =
+ verifyAndGetMissingDirs(masterCookie,
+ allowExpansion, journalDirectories);
+ missedCookieDirs.addAll(journalResult.getLeft());
+ existingCookies.addAll(journalResult.getRight());
+ // 4.2. verify the cookies in ledger directories
+ Pair<List<File>, List<Cookie>> ledgerResult =
+ verifyAndGetMissingDirs(masterCookie,
+ allowExpansion, allLedgerDirs);
+ missedCookieDirs.addAll(ledgerResult.getLeft());
+ existingCookies.addAll(ledgerResult.getRight());
+
+ // 5. if there are directories missing cookies,
+ // this is either a:
+ // - new environment
+ // - a directory is being added
+ // - a directory has been corrupted/wiped, which is an error
+ if (!missedCookieDirs.isEmpty()) {
+ if (rmCookie == null) {
+ // 5.1 new environment: all directories should be empty
+ verifyDirsForNewEnvironment(missedCookieDirs);
+ stampNewCookie(conf, masterCookie, rm, Version.NEW,
+ journalDirectories, allLedgerDirs);
+ } else if (allowExpansion) {
+ // 5.2 storage is expanding
+ Set<File> knownDirs = getKnownDirs(existingCookies);
+ verifyDirsForStorageExpansion(missedCookieDirs, knownDirs);
+ stampNewCookie(conf, masterCookie,
+ rm, rmCookie.getVersion(),
+ journalDirectories, allLedgerDirs);
+ } else {
+ // 5.3 Cookie-less directories and
+ // we can't do anything with them
+ LOG.error("There are directories without a cookie,"
+ + " and this is neither a new environment,"
+ + " nor is storage expansion enabled. "
+ + "Empty directories are {}", missedCookieDirs);
+ throw new InvalidCookieException();
}
- masterCookie.writeToRegistrationManager(rm, conf, rmCookie != null ? rmCookie.getVersion() : Version.NEW);
}
} catch (IOException ioe) {
LOG.error("Error accessing cookie on disks", ioe);
@@ -552,6 +496,55 @@ public static void checkEnvironmentWithStorageExpansion(
}
}
+ private static void verifyDirsForNewEnvironment(List<File> missedCookieDirs)
+ throws InvalidCookieException {
+ List<File> nonEmptyDirs = new ArrayList<>();
+ for (File dir : missedCookieDirs) {
+ String[] content = dir.list();
+ if (content != null && content.length != 0) {
+ nonEmptyDirs.add(dir);
+ }
+ }
+ if (!nonEmptyDirs.isEmpty()) {
+ LOG.error("Not all the new directories are empty. New directories that are not empty are: " + nonEmptyDirs);
+ throw new InvalidCookieException();
+ }
+ }
+
+ private static Set<File> getKnownDirs(List<Cookie> cookies) {
+ return cookies.stream()
+ .flatMap((c) -> Arrays.stream(c.getLedgerDirPathsFromCookie()))
+ .map((s) -> new File(s))
+ .collect(Collectors.toSet());
+ }
+
+ private static void verifyDirsForStorageExpansion(
+ List<File> missedCookieDirs,
+ Set<File> existingLedgerDirs) throws InvalidCookieException {
+
+ List<File> dirsMissingData = new ArrayList<File>();
+ List<File> nonEmptyDirs = new ArrayList<File>();
+ for (File dir : missedCookieDirs) {
+ if (existingLedgerDirs.contains(dir.getParentFile())) {
+ // if one of the existing ledger dirs doesn't have cookie,
+ // let us not proceed further
+ dirsMissingData.add(dir);
+ continue;
+ }
+ String[] content = dir.list();
+ if (content != null && content.length != 0) {
+ nonEmptyDirs.add(dir);
+ }
+ }
+ if (dirsMissingData.size() > 0 || nonEmptyDirs.size() > 0) {
+ LOG.error("Either not all local directories have cookies or directories being added "
+ + " newly are not empty. "
+ + "Directories missing cookie file are: " + dirsMissingData
+ + " New directories that are not empty are: " + nonEmptyDirs);
+ throw new InvalidCookieException();
+ }
+ }
+
/**
* Return the configured address of the bookie.
*/
@@ -651,16 +644,19 @@ public Bookie(ServerConfiguration conf, StatsLogger statsLogger)
ZooKeeper zooKeeper = null; // ZooKeeper is null existing only for testing
if (registrationManager != null) {
zooKeeper = ((ZKRegistrationManager) this.registrationManager).getZk();
+ // current the registration manager is zookeeper only
+ ledgerManagerFactory = LedgerManagerFactory.newLedgerManagerFactory(
+ conf,
+ zooKeeper);
+ LOG.info("instantiate ledger manager {}", ledgerManagerFactory.getClass().getName());
+ ledgerManager = ledgerManagerFactory.newLedgerManager();
+ } else {
+ ledgerManagerFactory = null;
+ ledgerManager = null;
}
- // current the registration manager is zookeeper only
- ledgerManagerFactory = LedgerManagerFactory.newLedgerManagerFactory(
- conf,
- zooKeeper);
} catch (KeeperException e) {
throw new MetadataStoreException("Failed to initialize ledger manager", e);
}
- LOG.info("instantiate ledger manager {}", ledgerManagerFactory.getClass().getName());
- ledgerManager = ledgerManagerFactory.newLedgerManager();
// Initialise ledgerDirMonitor. This would look through all the
// configured directories. When disk errors or all the ledger
@@ -1191,8 +1187,12 @@ synchronized int shutdown(int exitCode) {
// close Ledger Manager
try {
- ledgerManager.close();
- ledgerManagerFactory.uninitialize();
+ if (null != ledgerManager) {
+ ledgerManager.close();
+ }
+ if (null != ledgerManagerFactory) {
+ ledgerManagerFactory.uninitialize();
+ }
} catch (IOException ie) {
LOG.error("Failed to close active ledger manager : ", ie);
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
index d564254e1..40966cb47 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieShell.java
@@ -1759,6 +1759,7 @@ int runCmd(CommandLine cmdLine) {
}
try {
+ conf.setAllowStorageExpansion(true);
Bookie.checkEnvironmentWithStorageExpansion(conf, rm,
Lists.newArrayList(journalDirectories), allLedgerDirs);
} catch (BookieException e) {
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
index fe6e3475f..7399824a6 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/ScanAndCompareGarbageCollector.java
@@ -95,6 +95,12 @@ public ScanAndCompareGarbageCollector(LedgerManager ledgerManager, CompactableLe
@Override
public void gc(GarbageCleaner garbageCleaner) {
+ if (null == ledgerManager) {
+ // if ledger manager is null, the bookie is not started to connect to metadata store.
+ // so skip garbage collection
+ return;
+ }
+
try {
// Get a set of all ledgers on the bookie
NavigableSet<Long> bkActiveLedgers = Sets.newTreeSet(ledgerStorage.getActiveLedgersInRange(0,
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
index 1d1edaee3..8da34ef38 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieInitializationTest.java
@@ -354,31 +354,33 @@ public void testDuplicateBookieServerStartup() throws Exception {
*/
@Test
public void testBookieServerStartupOnEphemeralPorts() throws Exception {
- File tmpDir = createTempDir("bookie", "test");
+ File tmpDir1 = createTempDir("bookie", "test1");
+ File tmpDir2 = createTempDir("bookie", "test2");
- ServerConfiguration conf = TestBKConfiguration.newServerConfiguration();
- conf.setZkServers(null)
+ ServerConfiguration conf1 = TestBKConfiguration.newServerConfiguration();
+ conf1.setZkServers(null)
.setBookiePort(0)
- .setJournalDirName(tmpDir.getPath())
+ .setJournalDirName(tmpDir1.getPath())
.setLedgerDirNames(
- new String[] { tmpDir.getPath() });
- assertEquals(0, conf.getBookiePort());
-
- ServerConfiguration conf1 = new ServerConfiguration();
- conf1.addConfiguration(conf);
+ new String[] { tmpDir1.getPath() });
+ assertEquals(0, conf1.getBookiePort());
BookieServer bs1 = new BookieServer(conf1);
- conf.setZkServers(zkUtil.getZooKeeperConnectString());
- rm.initialize(conf, () -> {}, NullStatsLogger.INSTANCE);
+ conf1.setZkServers(zkUtil.getZooKeeperConnectString());
+ rm.initialize(conf1, () -> {}, NullStatsLogger.INSTANCE);
bs1.getBookie().registrationManager = rm;
bs1.start();
assertFalse(0 == conf1.getBookiePort());
// starting bk server with same conf
- ServerConfiguration conf2 = new ServerConfiguration();
- conf2.addConfiguration(conf);
+ ServerConfiguration conf2 = TestBKConfiguration.newServerConfiguration();
+ conf2.setZkServers(null)
+ .setBookiePort(0)
+ .setJournalDirName(tmpDir2.getPath())
+ .setLedgerDirNames(
+ new String[] { tmpDir2.getPath() });
BookieServer bs2 = new BookieServer(conf2);
RegistrationManager newRm = new ZKRegistrationManager();
- newRm.initialize(conf, () -> {}, NullStatsLogger.INSTANCE);
+ newRm.initialize(conf2, () -> {}, NullStatsLogger.INSTANCE);
bs2.getBookie().registrationManager = newRm;
bs2.start();
assertFalse(0 == conf2.getBookiePort());
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
index e817776b5..a634e2567 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
@@ -28,14 +28,13 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
-import com.google.common.base.Stopwatch;
import com.google.common.collect.Sets;
import java.io.File;
import java.io.IOException;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
-import java.util.concurrent.TimeUnit;
+import org.apache.bookkeeper.bookie.BookieException.InvalidCookieException;
import org.apache.bookkeeper.client.BookKeeperAdmin;
import org.apache.bookkeeper.conf.ClientConfiguration;
import org.apache.bookkeeper.conf.ServerConfiguration;
@@ -45,12 +44,12 @@
import org.apache.bookkeeper.stats.NullStatsLogger;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.apache.bookkeeper.test.PortManager;
+import org.apache.bookkeeper.util.BookKeeperConstants;
import org.apache.bookkeeper.util.IOUtils;
import org.apache.bookkeeper.versioning.LongVersion;
import org.apache.bookkeeper.versioning.Version;
import org.apache.bookkeeper.versioning.Versioned;
import org.apache.commons.io.FileUtils;
-import org.junit.After;
import org.junit.Assert;
import org.junit.Test;
@@ -186,6 +185,66 @@ public void testDirectoryMissing() throws Exception {
b.shutdown();
}
+ /**
+ * Test that if a cookie is missing from a journal directory
+ * the bookie will fail to start
+ */
+ @Test
+ public void testCookieMissingOnJournalDir() throws Exception {
+ String[] ledgerDirs = new String[] {
+ newDirectory(), newDirectory(), newDirectory() };
+ String journalDir = newDirectory();
+ ServerConfiguration conf = TestBKConfiguration.newServerConfiguration()
+ .setZkServers(zkUtil.getZooKeeperConnectString())
+ .setJournalDirName(journalDir)
+ .setLedgerDirNames(ledgerDirs)
+ .setBookiePort(bookiePort);
+
+ Bookie b = new Bookie(conf); // should work fine
+ b.start();
+ b.shutdown();
+
+ File cookieFile =
+ new File(Bookie.getCurrentDirectory(new File(journalDir)), BookKeeperConstants.VERSION_FILENAME);
+ assertTrue(cookieFile.delete());
+ try {
+ new Bookie(conf);
+ fail("Shouldn't have been able to start");
+ } catch (BookieException.InvalidCookieException ice) {
+ // correct behaviour
+ }
+ }
+
+ /**
+ * Test that if a cookie is missing from a journal directory
+ * the bookie will fail to start
+ */
+ @Test
+ public void testCookieMissingOnLedgerDir() throws Exception {
+ String[] ledgerDirs = new String[] {
+ newDirectory(), newDirectory(), newDirectory() };
+ String journalDir = newDirectory();
+ ServerConfiguration conf = TestBKConfiguration.newServerConfiguration()
+ .setZkServers(zkUtil.getZooKeeperConnectString())
+ .setJournalDirName(journalDir)
+ .setLedgerDirNames(ledgerDirs)
+ .setBookiePort(bookiePort);
+
+ Bookie b = new Bookie(conf); // should work fine
+ b.start();
+ b.shutdown();
+
+ File cookieFile =
+ new File(Bookie.getCurrentDirectory(new File(ledgerDirs[0])), BookKeeperConstants.VERSION_FILENAME);
+ assertTrue(cookieFile.delete());
+ try {
+ new Bookie(conf);
+ fail("Shouldn't have been able to start");
+ } catch (BookieException.InvalidCookieException ice) {
+ // correct behaviour
+ }
+ }
+
/**
* Test that if a directory is added to a
* preexisting bookie, the bookie will fail
@@ -543,10 +602,38 @@ public void testRestartWithHostNameAsBookieID() throws Exception {
b.shutdown();
conf.setUseHostNameAsBookieID(true);
- b = new Bookie(conf);
+ try {
+ new Bookie(conf);
+ fail("Should not start a bookie with hostname if the bookie has been started with an ip");
+ } catch (InvalidCookieException e) {
+ // expected
+ }
+ }
+
+ /**
+ * Test restart bookie with new advertisedAddress, which had cookie generated with ip.
+ */
+ @Test
+ public void testRestartWithAdvertisedAddressAsBookieID() throws Exception {
+ String[] ledgerDirs = new String[] { newDirectory(), newDirectory(),
+ newDirectory() };
+ String journalDir = newDirectory();
+ ServerConfiguration conf = TestBKConfiguration.newServerConfiguration()
+ .setZkServers(zkUtil.getZooKeeperConnectString())
+ .setJournalDirName(journalDir).setLedgerDirNames(ledgerDirs)
+ .setBookiePort(bookiePort);
+ conf.setUseHostNameAsBookieID(false);
+ Bookie b = new Bookie(conf); // should work fine
b.start();
- assertTrue("Fails to recognize bookie which was started with IPAddr as ID", !conf.getUseHostNameAsBookieID());
b.shutdown();
+
+ conf.setAdvertisedAddress("unknown");
+ try {
+ new Bookie(conf);
+ fail("Should not start a bookie with ip if the bookie has been started with an ip");
+ } catch (InvalidCookieException e) {
+ // expected
+ }
}
/**
@@ -568,10 +655,12 @@ public void testRestartWithIpAddressAsBookieID() throws Exception {
b.shutdown();
conf.setUseHostNameAsBookieID(false);
- b = new Bookie(conf);
- b.start();
- assertTrue("Fails to recognize bookie which was started with HostName as ID", conf.getUseHostNameAsBookieID());
- b.shutdown();
+ try {
+ new Bookie(conf);
+ fail("Should not start a bookie with ip if the bookie has been started with an ip");
+ } catch (InvalidCookieException e) {
+ // expected
+ }
}
/**
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpdateCookieCmdTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpdateCookieCmdTest.java
index c11fbca6e..72efcf2ab 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpdateCookieCmdTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpdateCookieCmdTest.java
@@ -26,17 +26,15 @@
import java.io.IOException;
import java.net.UnknownHostException;
import java.util.List;
-
+import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.discover.RegistrationManager;
import org.apache.bookkeeper.discover.ZKRegistrationManager;
-import org.apache.bookkeeper.stats.NullStatsLogger;
-import org.junit.Assert;
-
-import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.proto.BookieServer;
+import org.apache.bookkeeper.stats.NullStatsLogger;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.apache.bookkeeper.versioning.Version;
import org.apache.zookeeper.KeeperException;
+import org.junit.Assert;
import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -192,7 +190,7 @@ private void verifyCookieInZooKeeper(ServerConfiguration conf, int expectedCount
}
private void updateCookie(String option, String optionVal, boolean useHostNameAsBookieID) throws Exception {
- ServerConfiguration conf = bsConfs.get(0);
+ ServerConfiguration conf = new ServerConfiguration(bsConfs.get(0));
BookieServer bks = bs.get(0);
bks.shutdown();
diff --git a/tests/backward/src/test/java/org/apache/bookkeeper/tests/backward/TestBackwardCompat.java b/tests/backward/src/test/java/org/apache/bookkeeper/tests/backward/TestBackwardCompat.java
index b57230dd6..4101c0c58 100644
--- a/tests/backward/src/test/java/org/apache/bookkeeper/tests/backward/TestBackwardCompat.java
+++ b/tests/backward/src/test/java/org/apache/bookkeeper/tests/backward/TestBackwardCompat.java
@@ -588,7 +588,7 @@ public void testCompatReads() throws Exception {
// Start the current server, will not require a filesystem upgrade
ServerCurrent scur = new ServerCurrent(journalDir, ledgerDir, port,
- true);
+ false);
scur.start();
// check that old client can read its old ledgers on new server
@@ -626,7 +626,7 @@ public void testCompatWrites() throws Exception {
// Start the current server, will not require a filesystem upgrade
ServerCurrent scur = new ServerCurrent(journalDir, ledgerDir, port,
- true);
+ false);
scur.start();
// Check that current client can to write to server
@@ -686,7 +686,7 @@ public void testCompatHierarchicalLedgerManager() throws Exception {
s420.stop();
// Start the current server
- ServerCurrent scur = new ServerCurrent(journalDir, ledgerDir, port, true);
+ ServerCurrent scur = new ServerCurrent(journalDir, ledgerDir, port, false);
scur.getConf().setLedgerManagerFactoryClassName("org.apache.bookkeeper.meta.HierarchicalLedgerManagerFactory");
scur.getConf().setProperty(AbstractConfiguration.LEDGER_MANAGER_FACTORY_DISABLE_CLASS_CHECK, true);
scur.start();
----------------------------------------------------------------
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