ctubbsii commented on code in PR #3717:
URL: https://github.com/apache/accumulo/pull/3717#discussion_r1694335433
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -280,38 +280,42 @@ Fate<Manager> fate() {
/* UNLOAD_ROOT_TABLET */ {O, O, O, X, X, X, X},
/* STOP */ {O, O, O, O, O, X, X}};
//@formatter:on
- synchronized void setManagerState(ManagerState newState) {
- if (state.equals(newState)) {
+ synchronized void setManagerState(final ManagerState newState) {
+ if (state == newState) {
return;
}
if (!transitionOK[state.ordinal()][newState.ordinal()]) {
log.error("Programmer error: manager should not transition from {} to
{}", state, newState);
}
- ManagerState oldState = state;
+ final ManagerState oldState = state;
state = newState;
nextEvent.event("State changed from %s to %s", oldState, newState);
- if (newState == ManagerState.STOP) {
- // Give the server a little time before shutdown so the client
- // thread requesting the stop can return
- ScheduledFuture<?> future =
getContext().getScheduledExecutor().scheduleWithFixedDelay(() -> {
- // This frees the main thread and will cause the manager to exit
- clientService.stop();
- Manager.this.nextEvent.event("stopped event loop");
- }, 100L, 1000L, MILLISECONDS);
- ThreadPools.watchNonCriticalScheduledTask(future);
- }
-
- if (oldState != newState && (newState == ManagerState.HAVE_LOCK)) {
- upgradeCoordinator.upgradeZookeeper(getContext(), nextEvent);
- }
-
- if (oldState != newState && (newState == ManagerState.NORMAL)) {
- if (fateRef.get() != null) {
- throw new IllegalStateException("Access to Fate should not have been"
- + " initialized prior to the Manager finishing upgrades. Please
save"
- + " all logs and file a bug.");
- }
- upgradeMetadataFuture = upgradeCoordinator.upgradeMetadata(getContext(),
nextEvent);
+ switch (newState) {
+ case STOP:
+ // Give the server a little time before shutdown so the client
+ // thread requesting the stop can return
+ final var future =
getContext().getScheduledExecutor().scheduleWithFixedDelay(() -> {
+ // This frees the main thread and will cause the manager to exit
+ clientService.stop();
+ Manager.this.nextEvent.event("stopped event loop");
+ }, 100L, 1000L, MILLISECONDS);
+ ThreadPools.watchNonCriticalScheduledTask(future);
+ break;
+ case HAVE_LOCK:
+ upgradeCoordinator.upgradeZookeeper(getContext(), nextEvent);
+ break;
+ case NORMAL:
+ // fate isn't started until after upgrades are done, so non-null
fateRef implies isUpgrading
+ // should return false
+ if (fateRef.get() != null && isUpgrading()) {
Review Comment:
I ended up going the second route with this. It made things much nicer to
reason about, and I could use isUpgrading for more strict checks elsewhere.
--
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]
