You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/08/02 05:10:01 UTC
[GitHub] [ozone] errose28 opened a new pull request #2484: Hdds 5432 no downgrade action
errose28 opened a new pull request #2484:
URL: https://github.com/apache/ozone/pull/2484
## What changes were proposed in this pull request?
Enable downgrade testing from current version of Ozone to 1.1.0. This involves implementing the volume formatting changes needed on the datanode in a backwards compatible way.
Creating a PR for visibility, but leaving it as a draft until the following are completed:
- [ ] Descriptions of how corner cases have been handled to aid in reviewing.
- [ ] Unit testing of corner cases.
- [ ] Fix Interrupt on datanode shutdown that is failing some integration tests in TestHDDSUpgrade.
- [ ] Breakdown of changes to aid in reviewing.
## What is the link to the Apache JIRA
HDDS-5432
## How was this patch tested?
- Downgrade acceptance testing enabled.
- TODO: Unit testing for corner cases.
--
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
[GitHub] [ozone] adoroszlai commented on a change in pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by 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
[GitHub] [ozone] errose28 merged pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by GitBox <gi...@apache.org>.
errose28 merged pull request #2484:
URL: https://github.com/apache/ozone/pull/2484
--
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
[GitHub] [ozone] errose28 commented on pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2484:
URL: https://github.com/apache/ozone/pull/2484#issuecomment-914463854
Thanks for taking the time to review and approve this large PR @adoroszlai @avijayanhwx. Merging it now.
--
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
[GitHub] [ozone] errose28 commented on a change in pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by 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
[GitHub] [ozone] errose28 commented on pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #2484:
URL: https://github.com/apache/ozone/pull/2484#issuecomment-896377564
CI failure passed locally and seems unrelated. Retriggering.
--
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
[GitHub] [ozone] errose28 commented on a change in pull request #2484: HDDS-5432. Enable downgrade testing after 1.1.0 release.
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #2484:
URL: https://github.com/apache/ozone/pull/2484#discussion_r693144351
##########
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:
1.0.0 -> 1.1.0 did not require any manual intervention. So the upgrade framework supports upgrading from 1.0.0 (GA) to any later version.
--
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