keith-turner commented on code in PR #5277:
URL: https://github.com/apache/accumulo/pull/5277#discussion_r1928876933
##########
server/manager/src/main/java/org/apache/accumulo/manager/upgrade/PreUpgradeValidation.java:
##########
@@ -54,7 +64,14 @@ public void validate(final ServerContext context, final
EventCoordinator eventCo
log.debug("already at current data version: {}, skipping validation",
cv);
return;
}
- validateACLs(context);
+
+ try {
+ validateACLs(context);
+ abortIfFateTransactions(context);
+ validateProperties(context);
Review Comment:
> At a minimum, it sounds like we want the property validation to stay in
the upgradeMetadata method.
Unfortunately that seems like what we need to do for now. I liked what this
change was attempting to do. The following is the result of trying to puzzle
through how this could work, not suggesting any changes for this PR.
For property validation during upgrade it seems like we do not want to kill
the upgrade unless its known a property would compromise the upgrade itself
(like upgrade needs to bring the metadata table online, so any property that
would interfere w/ that is going to be a problem). For properties that are no
longer valid, but will not impact upgrade it seems like it would be best to
just handle fixing those up after the upgrade finishes. However it is probably
not possible to reliably know what invalid property will only impact upgrade.
So it seems like it would be nice to somehow validate prior to upgrade making
any changes, but that is not possible w/ the current code. Would probably need
to update the Upgrader interface to add new functionality to make that work.
For the current code, if upgrade does fail after upgrading zookeeper and
before upgrading metadata because of invalid properties then how does someone
move forward? Seems like this needs to be figured out in some way.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
