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_r150201065
########## File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java ########## @@ -441,77 +335,122 @@ private void checkIfDirsOnSameDiskPartition(List<File> dirs) throws DiskPartitio } } + static Versioned<Cookie> readAndVerifyCookieFromRegistrationManager(Cookie masterCookie, + ServerConfiguration conf, + RegistrationManager rm) + 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 { + // 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); + } + Versioned<Cookie> rmCookie = null; + for (BookieSocketAddress address : addresses) { + try { + 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 + if (conf.getAllowStorageExpansion()) { + masterCookie.verifyIsSuperSet(rmCookie.getValue()); + } else { + masterCookie.verify(rmCookie.getValue()); + } + } catch (CookieNotFoundException e) { + continue; + } + } + return rmCookie; + } + + private static Pair<List<File>, List<Cookie>> verifyAndGetMissingDirs(Cookie masterCookie, + ServerConfiguration conf, + 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 (conf.getAllowStorageExpansion()) { + masterCookie.verifyIsSuperSet(c); + } else { + masterCookie.verify(c); + } + existedCookies.add(c); + } catch (FileNotFoundException fnf) { + missingDirs.add(dir); + } + } + return Pair.of(missingDirs, existedCookies); + } + public static void checkEnvironmentWithStorageExpansion( ServerConfiguration conf, RegistrationManager rm, List<File> journalDirectories, List<File> allLedgerDirs) throws BookieException { 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); - } - } - + // 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(); - 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.verifyIsSuperSet(journalCookie); - } - } + // 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. + final Versioned<Cookie> rmCookie = readAndVerifyCookieFromRegistrationManager( + masterCookie, + conf, + rm + ); + final boolean newEnv = null == rmCookie; - for (File dir : allLedgerDirs) { - checkDirectoryStructure(dir); - try { - Cookie c = Cookie.readFromDirectory(dir); - masterCookie.verifyIsSuperSet(c); - } catch (FileNotFoundException fnf) { - missedCookieDirs.add(dir); - } + // 4. check if the cookie appear in all the directories. + List<File> missedCookieDirs = new ArrayList<>(); + List<Cookie> existedCookies = Lists.newArrayList(); + if (!newEnv) { + existedCookies.add(rmCookie.getValue()); } + // 4.1 verify the cookies in journal directories + Pair<List<File>, List<Cookie>> journalResult = + verifyAndGetMissingDirs(masterCookie, conf, journalDirectories); + missedCookieDirs.addAll(journalResult.getLeft()); + existedCookies.addAll(journalResult.getRight()); + // 4.2. verify the cookies in ledger directories + Pair<List<File>, List<Cookie>> ledgerResult = + verifyAndGetMissingDirs(masterCookie, conf, allLedgerDirs); + missedCookieDirs.addAll(ledgerResult.getLeft()); + existedCookies.addAll(ledgerResult.getRight()); + + // 5. if it is not a new environment, find out the real directories that miss cookie data 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()); + for (Cookie cookie : existedCookies) { + Collections.addAll(existingLedgerDirs, cookie.getLedgerDirPathsFromCookie()); } List<File> dirsMissingData = new ArrayList<File>(); List<File> nonEmptyDirs = new ArrayList<File>(); Review comment: rename to nonEmptyNewDirs ---------------------------------------------------------------- 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