[GitHub] [ozone] adoroszlai commented on a change in pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.

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

adoroszlai commented on a change in pull request #2484:
URL: https://github.com/apache/ozone/pull/2484#discussion_r691525623



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java
##########
@@ -452,8 +452,8 @@ public boolean isTransitionAllowedTo(DatanodeStates newState) {
   public void startDaemon() {
     Runnable startStateMachineTask = () -> {
       try {
-        start();
         LOG.info("Ozone container server started.");
+        start();

Review comment:
       Great, datanodes will no longer print "server started" right when being stopped. :)  This has always bothered me, but not enough to fix it.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -50,9 +51,13 @@
   protected TreeMap<Integer, T> features = new TreeMap<>();
   protected Map<String, T> featureMap = new HashMap<>();
   protected volatile Status currentUpgradeState = FINALIZATION_REQUIRED;
+  // Allows querying upgrade state while an upgrade is in progress.
+  // Note that MLV may have been incremented during the upgrade
+  // by the time the value is read/used.
+  private ReentrantReadWriteLock lock;
 
   protected void init(int version, T[] lfs) throws IOException {
-
+    lock = new ReentrantReadWriteLock();

Review comment:
       Nit: `lock` could be `final` and initialized at the declaration.  And `init()` may be guarded by the write lock.

##########
File path: hadoop-ozone/dist/src/main/compose/upgrade/test.sh
##########
@@ -32,12 +32,7 @@ run_test_scripts ${tests} || RESULT=$?
 RESULT_DIR="$ALL_RESULT_DIR" create_results_dir
 
 # Upgrade tests to be run.
-# Run all upgrades even if one fails.
-# Any failure will save a failing return code to $RESULT.
-set +e
-run_test manual-upgrade 0.5.0 1.1.0
-run_test non-rolling-upgrade 1.0.0 1.1.0

Review comment:
       I didn't see these tests (and old version-specific logic) being removed mentioned in the PR.  I'm fine with it, but maybe the README should be updated accordingly.  Specifically: _... testing upgrades from any previous version to another previous version_ is no longer true (if I understand it correctly).

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/HddsVolumeUtil.java
##########
@@ -168,18 +169,25 @@ public static String getProperty(
 
   /**
    * Check Volume is in consistent state or not.
+   * Prior to SCM HA, volumes used the format {@code <volume>/hdds/<scm-id>}.
+   * Post SCM HA, new volumes will use the format {@code <volume>/hdds/<cluster
+   * -id>}.
+   * Existing volumes using SCM ID would have been reformatted to have {@code
+   * <volume>/hdds/<cluster-id>} as a symlink pointing to {@code <volume
+   * >/hdds/<scm-id>}.
+   *
    * @param hddsVolume
-   * @param scmId
    * @param clusterId
    * @param logger
    * @return true - if volume is in consistent state, otherwise false.
    */
-  public static boolean checkVolume(HddsVolume hddsVolume, String scmId, String
-      clusterId, Logger logger) {
+  public static boolean checkVolume(HddsVolume hddsVolume, String scmId,
+      String clusterId, ConfigurationSource conf, Logger logger) {
     File hddsRoot = hddsVolume.getHddsRootDir();
     String volumeRoot = hddsRoot.getPath();
     File clusterDir = new File(hddsRoot, clusterId);
-    File scmDir = new File(hddsRoot, scmId);
+    String errorPrefix = "Volume " + volumeRoot + " is in an inconsistent " +
+        "state.";

Review comment:
       Nit: This is used only in one of the branches.  Should it be defined only there?




-- 
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: issues-unsubscribe@ozone.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org