kevinrr888 commented on code in PR #5531:
URL: https://github.com/apache/accumulo/pull/5531#discussion_r2072012218
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -638,6 +638,43 @@ public void run() {
*/
private Map<DataLevel,Set<KeyExtent>> partitionMigrations() {
final Map<DataLevel,Set<KeyExtent>> partitionedMigrations = new
EnumMap<>(DataLevel.class);
+
+ // Don't try to check migrations if the Root and Metadata
+ // tables are not hosted.
+ boolean skipMigrationCheck = false;
+
+ if (watchers.size() != 3) {
+ log.debug("Skipping migration check, not all TabletGroupWatchers are
started");
+ skipMigrationCheck = true;
+ } else {
+ for (TabletGroupWatcher watcher : watchers) {
+ if (!watcher.isAlive()) {
+ log.debug("Skipping migration check, not all TabletGroupWatchers are
started");
+ skipMigrationCheck = true;
+ break;
+ }
Review Comment:
I had a problem with one of the ITs in this case where the TGWs wouldn't be
started and it would hang on the scan. It hung every time then suddenly after a
few changes it stopped hanging, so I assumed it was fixed. Looking back, I
should have still been checking that the TGWs were started...
This is a good catch and looks good to me
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -638,6 +638,43 @@ public void run() {
*/
private Map<DataLevel,Set<KeyExtent>> partitionMigrations() {
final Map<DataLevel,Set<KeyExtent>> partitionedMigrations = new
EnumMap<>(DataLevel.class);
+
+ // Don't try to check migrations if the Root and Metadata
+ // tables are not hosted.
+ boolean skipMigrationCheck = false;
Review Comment:
I'm thinking we want to do this check everywhere we scan for migrations.
This is done in several places in Manager.
##########
server/manager/src/main/java/org/apache/accumulo/manager/Manager.java:
##########
@@ -638,6 +638,43 @@ public void run() {
*/
private Map<DataLevel,Set<KeyExtent>> partitionMigrations() {
final Map<DataLevel,Set<KeyExtent>> partitionedMigrations = new
EnumMap<>(DataLevel.class);
+
+ // Don't try to check migrations if the Root and Metadata
+ // tables are not hosted.
+ boolean skipMigrationCheck = false;
+
+ if (watchers.size() != 3) {
+ log.debug("Skipping migration check, not all TabletGroupWatchers are
started");
+ skipMigrationCheck = true;
+ } else {
+ for (TabletGroupWatcher watcher : watchers) {
+ if (!watcher.isAlive()) {
+ log.debug("Skipping migration check, not all TabletGroupWatchers are
started");
+ skipMigrationCheck = true;
+ break;
+ }
+ final DataLevel level = watcher.getLevel();
+ if (level == DataLevel.ROOT || level == DataLevel.METADATA) {
+ final TableId tid = level == DataLevel.ROOT ?
SystemTables.ROOT.tableId()
+ : SystemTables.METADATA.tableId();
Review Comment:
This use of `DataLevel` doesn't seem correct. See:
https://github.com/apache/accumulo/blob/f676217e757d823aff0499ff5af1beda2abc5f13/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java#L82-L85
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
