keith-turner commented on code in PR #5376:
URL: https://github.com/apache/accumulo/pull/5376#discussion_r1985614967
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -1259,18 +1246,65 @@ boolean canSuspendTablets() {
// setting.
return
getConfiguration().getBoolean(Property.MANAGER_METADATA_SUSPENDABLE);
}
- });
+ };
+ watchers.add(metadataTableTGW);
- watchers
- .add(new TabletGroupWatcher(this, this.rootTabletStore,
watchers.get(1), managerMetrics) {
+ final TabletGroupWatcher rootTableTGW =
+ new TabletGroupWatcher(this, this.rootTabletStore, watchers.get(1),
managerMetrics) {
@Override
boolean canSuspendTablets() {
// Never allow root tablet to enter suspended state.
return false;
}
- });
- for (TabletGroupWatcher watcher : watchers) {
- watcher.start();
+ };
+ watchers.add(rootTableTGW);
+
+ boolean rootTGWStarted = false;
+ boolean metaTGWStarted = false;
+ boolean userTGWStarted = false;
+
+ while (isUpgrading()) {
+ UpgradeStatus currentStatus = upgradeCoordinator.getStatus();
+ if (currentStatus == UpgradeStatus.FAILED || currentStatus ==
UpgradeStatus.COMPLETE) {
+ break;
+ }
+ switch (currentStatus) {
+ case UPGRADED_METADATA:
+ // Start processing user tables
+ userTableTGW.start();
+ userTGWStarted = true;
Review Comment:
Calling start() more than once will throw an exception and it seems like
this loop could do that. Could also use the getState method and maybe drop the
booleans.
```suggestion
if(userTableTGW.getState() == NEW){
userTableTGW.start();
}
```
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -1259,18 +1246,65 @@ boolean canSuspendTablets() {
// setting.
return
getConfiguration().getBoolean(Property.MANAGER_METADATA_SUSPENDABLE);
}
- });
+ };
+ watchers.add(metadataTableTGW);
- watchers
- .add(new TabletGroupWatcher(this, this.rootTabletStore,
watchers.get(1), managerMetrics) {
+ final TabletGroupWatcher rootTableTGW =
+ new TabletGroupWatcher(this, this.rootTabletStore, watchers.get(1),
managerMetrics) {
@Override
boolean canSuspendTablets() {
// Never allow root tablet to enter suspended state.
return false;
}
- });
- for (TabletGroupWatcher watcher : watchers) {
- watcher.start();
+ };
+ watchers.add(rootTableTGW);
+
+ boolean rootTGWStarted = false;
+ boolean metaTGWStarted = false;
+ boolean userTGWStarted = false;
+
+ while (isUpgrading()) {
+ UpgradeStatus currentStatus = upgradeCoordinator.getStatus();
+ if (currentStatus == UpgradeStatus.FAILED || currentStatus ==
UpgradeStatus.COMPLETE) {
+ break;
+ }
+ switch (currentStatus) {
+ case UPGRADED_METADATA:
+ // Start processing user tables
+ userTableTGW.start();
+ userTGWStarted = true;
+ break;
+ case UPGRADED_ROOT:
+ // Start processing the metadata table
+ metadataTableTGW.start();
+ metaTGWStarted = true;
+ break;
Review Comment:
The upgrade code may or may not block waiting on the TGW for each level, it
depends on if it reads something or not. So this code may not see each
UPGRADED_XXX. Maybe it needs to check the earlier TGW to be safe.
```suggestion
// TODO check if rootTableTGW needs to be started
break;
```
--
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]
