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/02/17 19:18:11 UTC
[GitHub] [ozone] avijayanhwx opened a new pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
avijayanhwx opened a new pull request #1933:
URL: https://github.com/apache/ozone/pull/1933
## What changes were proposed in this pull request?
- By default, a new cluster should use the latest layout version while being deployed (--init).
- If there is a new DN when the cluster is an unfinalized state, SCM layout version check will not let it become part of a pipeline until finalization.
- Refactored Layout version manager to give out default max version, remove "reset" API.
- Refactored Datanode layout version storage.
- Rename Layout feature in HDDS appropriately.
- Removed mock features in OM.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4817
## How was this patch tested?
Manually tested.
CI failures being looked into.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583219859
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
##########
@@ -211,15 +211,17 @@ public void testScmLayoutOnRegister()
throws IOException, InterruptedException, AuthenticationException {
try (SCMNodeManager nodeManager = createNodeManager(getConf())) {
- Integer nodeManagerSoftwareLayoutVersion =
- nodeManager.getLayoutVersionManager().getSoftwareLayoutVersion();
- LayoutVersionProto layoutInfoSuccess = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion).build();
- LayoutVersionProto layoutInfoFailure = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion + 1)
- .build();
+ HDDSLayoutVersionManager layoutVersionManager =
+ nodeManager.getLayoutVersionManager();
+ int nodeManagerMetadataLayoutVersion =
+ layoutVersionManager.getMetadataLayoutVersion();
+ int nodeManagerSoftwareLayoutVersion =
+ layoutVersionManager.getSoftwareLayoutVersion();
+ LayoutVersionProto layoutInfoSuccess = toLayoutVersionProto(
+ nodeManagerMetadataLayoutVersion, nodeManagerSoftwareLayoutVersion);
+ LayoutVersionProto layoutInfoFailure = toLayoutVersionProto(
+ nodeManagerSoftwareLayoutVersion + 1,
Review comment:
I was trying to mimic a DN from a future version here (with a new layout feature). I guess either is good, since registration only checks for SLV equality.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583204795
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDataNodeStartupSlvLessThanMlv.java
##########
@@ -48,8 +50,7 @@
public void testStartupSlvLessThanMlv() throws Exception {
// Add subdirectories under the temporary folder where the version file
// will be placed.
- File datanodeSubdir = tempFolder.newFolder("datanodeStorageConfig",
- "current");
+ File datanodeSubdir = tempFolder.newFolder(DATANODE_LAYOUT_VERSION_DIR);
OzoneConfiguration conf = new OzoneConfiguration();
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
Review comment:
I can't comment on the actual code since its unchanged, but it looks like the for loop on line 61 could be replaced with the new `HDDSLayoutVersionManager#maxLayoutVersion` method?
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/HDDSVolumeLayoutVersion.java
##########
@@ -21,11 +21,11 @@
* Datanode layout version which describes information about the layout version
* on the datanode.
*/
Review comment:
Nit: Update doc string since this class now deals with volumes, not the whole datanode.
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
##########
@@ -211,15 +211,17 @@ public void testScmLayoutOnRegister()
throws IOException, InterruptedException, AuthenticationException {
try (SCMNodeManager nodeManager = createNodeManager(getConf())) {
- Integer nodeManagerSoftwareLayoutVersion =
- nodeManager.getLayoutVersionManager().getSoftwareLayoutVersion();
- LayoutVersionProto layoutInfoSuccess = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion).build();
- LayoutVersionProto layoutInfoFailure = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion + 1)
- .build();
+ HDDSLayoutVersionManager layoutVersionManager =
+ nodeManager.getLayoutVersionManager();
+ int nodeManagerMetadataLayoutVersion =
+ layoutVersionManager.getMetadataLayoutVersion();
+ int nodeManagerSoftwareLayoutVersion =
+ layoutVersionManager.getSoftwareLayoutVersion();
+ LayoutVersionProto layoutInfoSuccess = toLayoutVersionProto(
+ nodeManagerMetadataLayoutVersion, nodeManagerSoftwareLayoutVersion);
+ LayoutVersionProto layoutInfoFailure = toLayoutVersionProto(
+ nodeManagerSoftwareLayoutVersion + 1,
Review comment:
Should this be `nodeManagerMetadataLayoutVersion`?
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -40,33 +42,38 @@
public abstract class AbstractLayoutVersionManager<T extends LayoutFeature>
implements LayoutVersionManager, LayoutVersionManagerMXBean {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(AbstractLayoutVersionManager.class);
+
protected int metadataLayoutVersion; // MLV.
protected int softwareLayoutVersion; // SLV.
protected TreeMap<Integer, T> features = new TreeMap<>();
protected Map<String, T> featureMap = new HashMap<>();
- protected volatile boolean isInitialized = false;
- protected volatile Status currentUpgradeState =
- FINALIZATION_REQUIRED;
+ protected volatile Status currentUpgradeState = FINALIZATION_REQUIRED;
protected void init(int version, T[] lfs) throws IOException {
- if (!isInitialized) {
- metadataLayoutVersion = version;
- initializeFeatures(lfs);
- softwareLayoutVersion = features.lastKey();
- isInitialized = true;
- if (softwareIsBehindMetaData()) {
- throw new IOException(
- String.format("Cannot initialize VersionManager. Metadata " +
- "layout version (%d) > software layout version (%d)",
- metadataLayoutVersion, softwareLayoutVersion));
- } else if (metadataLayoutVersion == softwareLayoutVersion) {
- currentUpgradeState = ALREADY_FINALIZED;
- }
+ metadataLayoutVersion = version;
+ initializeFeatures(lfs);
+ softwareLayoutVersion = features.lastKey();
+ if (softwareIsBehindMetaData()) {
+ throw new IOException(
+ String.format("Cannot initialize VersionManager. Metadata " +
+ "layout version (%d) > software layout version (%d)",
+ metadataLayoutVersion, softwareLayoutVersion));
+ } else if (metadataLayoutVersion == softwareLayoutVersion) {
+ currentUpgradeState = ALREADY_FINALIZED;
}
+ LayoutFeature mlvFeature = features.get(metadataLayoutVersion);
+ LayoutFeature slvFeature = features.get(softwareLayoutVersion);
+ LOG.info("Initializing Layout version manager with metadata layout" +
+ " = {} (version = {}), software layout = {} (version = {})",
+ mlvFeature, mlvFeature.layoutVersion(),
+ slvFeature, slvFeature.layoutVersion());
Review comment:
Logging mlvFeature and slvFeature is just going to give an object ID when logged, right? We want the description or something else that gives a meaningful String?
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
##########
@@ -696,7 +702,12 @@ void initializeOmStorage(OMStorage omStorage) throws IOException {
protected OzoneManager createOM()
Review comment:
Looks like this doesn't get called by MiniOzoneHACluster, which uses createOMService() instead. Probably need a minor modification in that class to call this so we can set version in HA clusters.
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureUtil.java
##########
@@ -39,7 +37,7 @@
* "disallowed" by just adding the following annotation, thereby keeping the
* method logic and upgrade logic separate.
*/
- @DisallowedUntilLayoutVersion(ERASURE_CODING)
+ //@DisallowedUntilLayoutVersion(ERASURE_CODING)
Review comment:
Looks like this class is being used for demonstration/testing purposes only. Should we move it out of *src* and under *test* with the test that uses it?
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
errose28 commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583234719
##########
File path: hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java
##########
@@ -211,15 +211,17 @@ public void testScmLayoutOnRegister()
throws IOException, InterruptedException, AuthenticationException {
try (SCMNodeManager nodeManager = createNodeManager(getConf())) {
- Integer nodeManagerSoftwareLayoutVersion =
- nodeManager.getLayoutVersionManager().getSoftwareLayoutVersion();
- LayoutVersionProto layoutInfoSuccess = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion).build();
- LayoutVersionProto layoutInfoFailure = LayoutVersionProto.newBuilder()
- .setMetadataLayoutVersion(1)
- .setSoftwareLayoutVersion(nodeManagerSoftwareLayoutVersion + 1)
- .build();
+ HDDSLayoutVersionManager layoutVersionManager =
+ nodeManager.getLayoutVersionManager();
+ int nodeManagerMetadataLayoutVersion =
+ layoutVersionManager.getMetadataLayoutVersion();
+ int nodeManagerSoftwareLayoutVersion =
+ layoutVersionManager.getSoftwareLayoutVersion();
+ LayoutVersionProto layoutInfoSuccess = toLayoutVersionProto(
+ nodeManagerMetadataLayoutVersion, nodeManagerSoftwareLayoutVersion);
+ LayoutVersionProto layoutInfoFailure = toLayoutVersionProto(
+ nodeManagerSoftwareLayoutVersion + 1,
Review comment:
I see, makes sense.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583228186
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeatureUtil.java
##########
@@ -39,7 +37,7 @@
* "disallowed" by just adding the following annotation, thereby keeping the
* method logic and upgrade logic separate.
*/
- @DisallowedUntilLayoutVersion(ERASURE_CODING)
+ //@DisallowedUntilLayoutVersion(ERASURE_CODING)
Review comment:
Moved 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.
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] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583227996
##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/HDDSVolumeLayoutVersion.java
##########
@@ -21,11 +21,11 @@
* Datanode layout version which describes information about the layout version
* on the datanode.
*/
Review comment:
Fixed it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#issuecomment-786760602
Thanks for the review @prashantpogde & @errose28.
----------------------------------------------------------------
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
---------------------------------------------------------------------
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 #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
errose28 commented on pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#issuecomment-786694052
LGTM +1
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583228097
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
##########
@@ -696,7 +702,12 @@ void initializeOmStorage(OMStorage omStorage) throws IOException {
protected OzoneManager createOM()
Review comment:
Good catch, fixed it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583221029
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractLayoutVersionManager.java
##########
@@ -40,33 +42,38 @@
public abstract class AbstractLayoutVersionManager<T extends LayoutFeature>
implements LayoutVersionManager, LayoutVersionManagerMXBean {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(AbstractLayoutVersionManager.class);
+
protected int metadataLayoutVersion; // MLV.
protected int softwareLayoutVersion; // SLV.
protected TreeMap<Integer, T> features = new TreeMap<>();
protected Map<String, T> featureMap = new HashMap<>();
- protected volatile boolean isInitialized = false;
- protected volatile Status currentUpgradeState =
- FINALIZATION_REQUIRED;
+ protected volatile Status currentUpgradeState = FINALIZATION_REQUIRED;
protected void init(int version, T[] lfs) throws IOException {
- if (!isInitialized) {
- metadataLayoutVersion = version;
- initializeFeatures(lfs);
- softwareLayoutVersion = features.lastKey();
- isInitialized = true;
- if (softwareIsBehindMetaData()) {
- throw new IOException(
- String.format("Cannot initialize VersionManager. Metadata " +
- "layout version (%d) > software layout version (%d)",
- metadataLayoutVersion, softwareLayoutVersion));
- } else if (metadataLayoutVersion == softwareLayoutVersion) {
- currentUpgradeState = ALREADY_FINALIZED;
- }
+ metadataLayoutVersion = version;
+ initializeFeatures(lfs);
+ softwareLayoutVersion = features.lastKey();
+ if (softwareIsBehindMetaData()) {
+ throw new IOException(
+ String.format("Cannot initialize VersionManager. Metadata " +
+ "layout version (%d) > software layout version (%d)",
+ metadataLayoutVersion, softwareLayoutVersion));
+ } else if (metadataLayoutVersion == softwareLayoutVersion) {
+ currentUpgradeState = ALREADY_FINALIZED;
}
+ LayoutFeature mlvFeature = features.get(metadataLayoutVersion);
+ LayoutFeature slvFeature = features.get(softwareLayoutVersion);
+ LOG.info("Initializing Layout version manager with metadata layout" +
+ " = {} (version = {}), software layout = {} (version = {})",
+ mlvFeature, mlvFeature.layoutVersion(),
+ slvFeature, slvFeature.layoutVersion());
Review comment:
Enum toString returns the "name" of the element, which in this case is the Layout Feature name.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on a change in pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on a change in pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#discussion_r583228269
##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDataNodeStartupSlvLessThanMlv.java
##########
@@ -48,8 +50,7 @@
public void testStartupSlvLessThanMlv() throws Exception {
// Add subdirectories under the temporary folder where the version file
// will be placed.
- File datanodeSubdir = tempFolder.newFolder("datanodeStorageConfig",
- "current");
+ File datanodeSubdir = tempFolder.newFolder(DATANODE_LAYOUT_VERSION_DIR);
OzoneConfiguration conf = new OzoneConfiguration();
conf.set(HddsConfigKeys.OZONE_METADATA_DIRS,
Review comment:
Fixed it.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx commented on pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx commented on pull request #1933:
URL: https://github.com/apache/ozone/pull/1933#issuecomment-782786599
cc @errose28 for review.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] avijayanhwx merged pull request #1933: HDDS-4817. Fresh deploy of Ozone must use the highest layout version by default.
Posted by GitBox <gi...@apache.org>.
avijayanhwx merged pull request #1933:
URL: https://github.com/apache/ozone/pull/1933
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org