This is an automated email from the ASF dual-hosted git repository.
sijie pushed a commit to branch branch-4.6
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 0db0cf6181f4949fe4684770bb81442694a24b27
Author: Sijie Guo <si...@apache.org>
AuthorDate: Wed Nov 29 18:53:39 2017 -0800
ISSUE #544: Bootup cookie validation considers an empty journal to signify
a new bookie
Descriptions of the changes in this PR:
- read cookie from registration manager as the source-of-truth (for all
potential options)
- remove duplicated code between checkEnvironment and
checkEnvironmentWithStorageExpansion
Author: Sijie Guo <si...@apache.org>
Author: Ivan Kelly <iv...@apache.org>
Reviewers: Ivan Kelly <iv...@apache.org>, Enrico Olivelli
<eolive...@gmail.com>, Venkateswararao Jujjuri (JV) <None>
This closes #712 from sijie/fix_cookie_issue, closes #544
---
.../java/org/apache/bookkeeper/bookie/Bookie.java | 424 ++++++++++-----------
.../org/apache/bookkeeper/bookie/BookieShell.java | 1 +
.../bookie/ScanAndCompareGarbageCollector.java | 6 +
.../bookie/BookieInitializationTest.java | 30 +-
.../org/apache/bookkeeper/bookie/CookieTest.java | 107 +++++-
.../bookkeeper/bookie/UpdateCookieCmdTest.java | 10 +-
.../tests/backward/TestBackwardCompat.java | 6 +-
7 files changed, 340 insertions(+), 244 deletions(-)
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 83888c1..0983395 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 static
org.apache.bookkeeper.bookie.BookKeeperServerStats.WRITE_BYTES;
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.net.UnknownHostException;
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 java.util.stream.Collectors;
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.bookkeeper.versioning.Versioned;
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 @@ public class Bookie extends BookieCriticalThread {
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 @@ public class Bookie extends BookieCriticalThread {
}
}
- 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 class Bookie extends BookieCriticalThread {
}
}
+ 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 class Bookie extends BookieCriticalThread {
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 @@ public class Bookie extends BookieCriticalThread {
// 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 d564254..40966cb 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 @@ public class BookieShell implements Tool {
}
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 fe6e347..7399824 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 class ScanAndCompareGarbageCollector implements
GarbageCollector{
@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 1d1edae..8da34ef 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 class BookieInitializationTest extends
BookKeeperClusterTestCase {
*/
@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 e817776..a634e25 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.apache.bookkeeper.bookie.UpgradeTest.newV2LedgerDirectory;
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.discover.ZKRegistrationManager;
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;
@@ -187,6 +186,66 @@ public class CookieTest extends BookKeeperClusterTestCase {
}
/**
+ * 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
* to start
@@ -543,10 +602,38 @@ public class CookieTest extends BookKeeperClusterTestCase
{
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 class CookieTest extends BookKeeperClusterTestCase
{
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 c11fbca..72efcf2 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.File;
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 @@ public class UpdateCookieCmdTest extends
BookKeeperClusterTestCase {
}
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 b57230d..4101c0c 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 class TestBackwardCompat {
// 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 class TestBackwardCompat {
// 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 class TestBackwardCompat {
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();
--
To stop receiving notification emails like this one, please contact
"commits@bookkeeper.apache.org" <commits@bookkeeper.apache.org>.
