[GitHub] [accumulo] EdColeman commented on a diff in pull request #3137: Removing replication info necessary for upgrade - resolves issue #3097

[GitBox <gi...@apache.org>]

EdColeman commented on code in PR #3137:
URL: https://github.com/apache/accumulo/pull/3137#discussion_r1062522508


##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/UpgradeCoordinator.java:
##########
@@ -179,15 +186,22 @@ public synchronized Future<Void> upgradeMetadata(ServerContext context,
           .submit(() -> {
             try {
               for (int v = currentVersion; v < AccumuloDataVersion.get(); v++) {
-                log.info("Upgrading Root from data version {}", v);
-                upgraders.get(v).upgradeRoot(context);
+                log.info("Upgrading Root - current version {} as step towards target version {}", v,
+                    AccumuloDataVersion.get());
+                if (upgraders.get(v) != null) {
+                  upgraders.get(v).upgradeRoot(context);
+                }

Review Comment:
   5436f22281  changes the null check to match  the ZooKeeper  sectionn - using rquireNonNull - while the check may be redundant if the previous check(s) pass it isolates issues with execution order.  The reason behind the check is to provide a better error message - during development a misconfiguration was throwing a null pointer and having a log statement helped.  
   
   Originally, the code skipped that section if null and it was pointed out by @ctubbsii that it would be better to fail.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org