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_r150196445
 
 

 ##########
 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);
 
 Review comment:
   It's kinda weird that we're not building the cookie from the passed in 
journalDirectories and ledgerDirs, but rather rebuilding these from the conf 
within generateCookie.

----------------------------------------------------------------
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

Reply via email to