[GitHub] [accumulo] keith-turner commented on pull request #3230: remove invalid tserver locks before upgrade

["keith-turner (via GitHub)" <gi...@apache.org>]

keith-turner commented on PR #3230:
URL: https://github.com/apache/accumulo/pull/3230#issuecomment-1538685122

   It may be good to move the pre upgrade checks into the Upgrader because the current version of Accumulo may not know how to read the previous version of Accumulo's metadata in order to perform upgrade checks.  Adding new methods make this step explicit when someone is thinking about writing a new Upgrader, it forces them to explicity consider if conditions are safe for an upgrade.  What conditions are safe for an upgrade could be highly dependent on the two versions of Accumulo.  If its an explicit step in the Upgrader, then it makes people consider upgrade safety in future versions of Accumulo.  With all of this in mind I made the following attempt to update the Upgrader interface.
   
   ```java
   public interface Upgrader {
   
     /**
      * This method is called before {@link #upgradeZookeeper(ServerContext)} to ensure the system is
      * in a suitable state to upgrade. Examples of what this check could attempt to do are :
      *
      * <UL>
      * <LI>If root tablet metadata is not upgraded, ensure the root tablet is not currently assigned
      * to a live tserver.</LI>
      * <LI>Ensure there are no server processes running from a previous version of Accumulo</LI>
      * </UL>
      *
      * <P>
      * The implementation of this method is delegated to the Uprader because it may have to read data
      * from DFS and Zookeeper in formats used by previous versions of Accumulo. This code for reading
      * previous storage formats should always be delegated to the uprader.
      * </P>
      *
      * <p>
      * Any possible conditions in the distributed system that could cause the upgrade process to
      * corrupt metadata or data should be checked for here and fail if detected.
      * </p>
      *
      * @throws IllegalStateException if conditions are not suitable for upgrade. The exception message
      *         should clearly indicate specifics about what is blocking this upgrade step.
      */
     void preUpgradeZookeeperCheck(ServerContext context);
   
     /**
      * Update entries in ZooKeeper - called before the root tablet is loaded.
      *
      * @param context the server context.
      */
     void upgradeZookeeper(ServerContext context);
   
     /**
      * This method is called before {@link #upgradeRoot(ServerContext)} to ensure the system is in a
      * suitabl state to upgrade. Examples of what this check could attempt to do are :
      *
      * <UL>
      * <LI>If metadata tablets metadata is not upgraded, ensure no metadata tablets are not currently
      * assigned to a live tserver</LI>
      * </UL>
      *
      * @throws IllegalStateException if conditions are not suitable for upgrade. The exception message
      *         should clearly indicate specifics about what is blocking this upgrade step.
      */
     void preUpgradeRootCheck(ServerContext context);
   
     /**
      * Update the root tablet - called after the root tablet is loaded and before the metadata table
      * is loaded.
      *
      * @param context the server context.
      */
     void upgradeRoot(ServerContext context);
   
     /**
      * This method is called before {@link #upgradeMetadata(ServerContext)} to ensure the system is in
      * a suitable state to upgrade. Examples of what this check could attempt to do are :
      *
      * <UL>
      * <LI>If user tablet metadata is not upgraded, ensure no user tablets are not currently assigned
      * to a live tserver</LI>
      * </UL>
      *
      * @throws IllegalStateException if conditions are not suitable for upgrade. The exception message
      *         should clearly indicate specifics about what is blocking this upgrade step.
      */
     void preUpgradeMetadataCheck(ServerContext context);
   
     /**
      * Update the metadata table - called after the metadata table is loaded and before loading user
      * tablets.
      *
      * @param context the server context.
      */
     void upgradeMetadata(ServerContext context);
   }
   
   ```
   
   Would the checks written so far in the PR fit into this model?
   
   


-- 
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-unsubscribe@accumulo.apache.org

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