ivankelly commented on a change in 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#discussion_r152213455
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
##########
@@ -441,117 +336,220 @@ 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
Review comment:
no, verifyIsSuperSet() only checks if the configured directories are a
superset of the directories in the previous cookie.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
