[GitHub] [accumulo] keith-turner commented on pull request #1751: WIP Design: Interface for handling tablet state migration

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

keith-turner commented on pull request #1751:
URL: https://github.com/apache/accumulo/pull/1751#issuecomment-715625908


   Is the following the code you are trying to simplify?  
   
   https://github.com/apache/accumulo/blob/353c611891a8b88b307dcba5e884467d4191b1f2/server/manager/src/main/java/org/apache/accumulo/master/TabletGroupWatcher.java#L252-L343
   
   How much of your goals could accomplished by replacing code blocks w/ functions like below?   I think you are also trying to simplify master.getGoalState() and possibly consolidate concerns that currently straddle master.getGoalState() and TabletGroupWatcher?  If that is true, then it seems like the following would not accomplish all of your goals.
   
   ```java
             if (goal == TabletGoalState.HOSTED) {
               if (state != TabletState.HOSTED && !tls.walogs.isEmpty()) {
                 if (master.recoveryManager.recoverLogs(tls.extent, tls.walogs))
                   continue;
               }
               switch (state) {
                 case HOSTED:
                   if (server.equals(master.migrations.get(tls.extent)))
                     master.migrations.remove(tls.extent);
                   break;
                 case ASSIGNED_TO_DEAD_SERVER:
                   hostTabletAssignedToDeadServer(...);
                   break;
                 case SUSPENDED:
                   hostSuspendedTablet(...);
                   break;
                 case UNASSIGNED:
                   hostUnassignedTablet(....);
                   break;
                 case ASSIGNED:
                   hostAssignedTablet(...);
                   break;
               }
             } else {
               switch (state) {
                 case SUSPENDED:
                   unassignSuspendedTablet(...);
                   break;
                 case UNASSIGNED:
                   cancelOfflineTableMigrations(tls);
                   break;
                 case ASSIGNED_TO_DEAD_SERVER:
                   unassignAssignedToDeadServerTablet(...);
                   break;
                 case HOSTED:
                   unassignHostedTablet(....);
                   break;
                 case ASSIGNED:
                   break;
               }
             }
             counts[state.ordinal()]++;
           }
   ```
   
   As for the merge code, that code straddles FATE and the Tablet goal state code in a way that is hard to follow.  It may need improvement, but I am uncertain what the improvement is.


----------------------------------------------------------------
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.

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