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

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

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



##########
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:
       Fixed. Also made all members private so child classes can't modify them without having the lock.

##########
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:
       I didn't mean to commit this and don't even remember why I changed it in the first place. Debugging something maybe. But if its good then I'll leave it.

##########
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:
       Looks like a leftover after refactoring. I'll just add it to the error message where its used.

##########
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:
       Yes I forgot to update the main README for the acceptance tests. I only updated the manual upgrade README. I will update the main one as well as the PR description since this is an important change.
   
   We can't do manual upgrade now that we have the upgrade framework, otherwise all upgrade actions would need to be re-implemented as shell scripts for the test to run, which is unnecessary. Manual upgrade will only work from one previously released version to another previously released version, which we do not need to test.




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