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 2022/10/17 05:56:05 UTC

[GitHub] [ozone] ChenSammi opened a new pull request, #3841: HDDS-7320. Enable Container SchemaV3 by default.

ChenSammi opened a new pull request, #3841:
URL: https://github.com/apache/ozone/pull/3841

   https://issues.apache.org/jira/browse/HDDS-7320 


-- 
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] captainzmc commented on pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on PR #3841:
URL: https://github.com/apache/ozone/pull/3841#issuecomment-1295892533

   Thanks @ChenSammi working for this.  SchemaV3 is a very important feature that we wanted to turn on by default in the release of 1.3.  Hi @errose28, would you take another look? 


-- 
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] ChenSammi commented on a diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r999122835


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   This test doesn't need to be updated every time there is a new layout feature.  The failure is "DB should not be null".   
   
   When schemaV3 is enabled, it requires a RocksDB created prior to write container metadata to DB.  The RocksDB is created when the first time DN registers to SCM, or cluster is upgrading from prior layout to DATANODE_SCHEMA_V3 layout. 
   
   This test is testing upgrade from initial layout to ScmHA layout.  The RocksDB doesn't get either chance to be created. So when putBlock is called, it fails.  Disable DATANODE_SCHEMA_V3 just keeps the original intent of this 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] ChenSammi commented on pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on PR #3841:
URL: https://github.com/apache/ozone/pull/3841#issuecomment-1297937759

   Thanks @errose28 , @ayushtkn , @captainzmc for the code 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.

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 diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
errose28 commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1003901404


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   > But in this test, it only upgrades like from 1.0.0 to 1.1.0, only the SCM-HA related finalization action is executed,
   
   Finalization always causes the component to go from its current layout version to the latest one. There is no way to finalize just up to the SCM HA layout feature, so the DN is also being finalized for schema V3 and the upgrade action for Schema V3 is being run. See the test logs
   ```log
   2022-10-24 16:49:59,462 [Listener at localhost/61869] INFO  upgrade.DatanodeSchemaV3FinalizeAction (DatanodeSchemaV3FinalizeAction.java:execute(50)) - Upgrading Datanode volume layout for Schema V3 support.
   ```
   
   I tried this test out locally and the failure is in `TestDatanodeUpgradeToScmHA#testFailedVolumeDuringFinalization`. In this test, a volume is failed while the datanode is finalizing, so its disk layout does not get upgraded. When the volume is restored and the DN is restarted, it looks like the volume is not formatted to schema v3 since it was out during finalization, causing the error.
   
   Unfortunately the upgrade framework currently only tracks layout version for a whole component, not on a per volume basis. This means if a volume is out while the DN is finalized, some manual startup logic must be added to make sure it gets formatted if it is restored. There is a [jira to improve this](https://issues.apache.org/jira/browse/HDDS-5475) but for now layout features handling volume format changes need to handle this case manually. For the SCM HA SCM ID to cluster ID change, this is done by calling `VersionedDatanodeFeatures.ScmHA.upgradeVolumeIfNeeded(volume, clusterId)` in `StorageVolumeUtil#checkVolume`. It looks like Schema V3 may need a similar thing to create the RocksDB on startup after the cluster ID has been obtained and the volume formatted with 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.

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] ChenSammi commented on a diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1002845044


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   @errose28, the flow you described is correct and works.  The key point is rocksDB is created during upgrade from 1.1.0 to 1.3.0 (1.2.1 in the middle) when Schema V3 finalization action is executed. 
   But in this test, it only upgrades like from 1.0.0 to 1.1.0,  only the SCM-HA related finalization action is executed, using cluster ID instead of scm ID as the data root directory.  Schema V3 finalization action will not executed, there is no change to create rocksDB for every volume.  In this case, if Schema V3 is enabled, then you can image what will happen.  Do I explain the difference clearly enough 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] ChenSammi commented on a diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1002845044


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   @errose28, the flow you described is correct and works.  The key point is rocksDB is created during upgrade from 1.1.0 to 1.3.0 (1.2.1 in the middle) when Schema V3 finalization action is executed. 
   But in this test, it only upgrades like from 1.0.0 to 1.1.0,  only the SCM-HA related finalization action is executed, using cluster ID instead of scm ID as the data root directory.  Schema V3 finalization action will not executed, there is no change to create rocksDB for every volume.  In this case, if Schema V3 is enabled, then you can image what will happen.  Not sure if I explain the difference clearly enough 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] captainzmc commented on pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on PR #3841:
URL: https://github.com/apache/ozone/pull/3841#issuecomment-1297935944

   Thanks @ChenSammi for the patch and thanks @errose28 @ayushtkn for the review. Let's merge this.


-- 
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] captainzmc merged pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
captainzmc merged PR #3841:
URL: https://github.com/apache/ozone/pull/3841


-- 
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] ChenSammi commented on a diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1006812459


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   @errose28 , thanks for the detail explain.  I now aware that all Finalization actions will be executed.  I reverted the changes in this test class with new commit.  



-- 
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 diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
errose28 commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r998584227


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   What was the error seen here that led to this line being added? This test should not need to be updated every time we add a new layout feature.



-- 
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] ChenSammi commented on a diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1002845044


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   @errose28, the flow you described is correct and works.  The key point is rocksDB is created during upgrade from 1.1.0 to 1.3.0 (1.2.1 in the middle) when Schema V3 finalization action is executed. 
   But in this test, it only upgrades like from 1.0.0 to 1.1.0,  only the SCM-HA related finalization action is executed, using cluster ID instead of scm ID as the data root directory.  Schema V3 finalization action will not executed, there is no change to create rocksDB for every volume.  In this case, if Schema V3 is enabled, then you can image what will happen.  You understand the difference 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 diff in pull request #3841: HDDS-7320. Enable Container SchemaV3 by default.

Posted by GitBox <gi...@apache.org>.
errose28 commented on code in PR #3841:
URL: https://github.com/apache/ozone/pull/3841#discussion_r1001073471


##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/upgrade/TestDatanodeUpgradeToScmHA.java:
##########
@@ -103,6 +104,8 @@ public TestDatanodeUpgradeToScmHA(boolean scmHAAlreadyEnabled) {
     this.scmHAAlreadyEnabled = scmHAAlreadyEnabled;
     conf = new OzoneConfiguration();
     conf.setBoolean(ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY, scmHAAlreadyEnabled);
+    // DATANODE_SCHEMA_V3 has higher feature version than SCM_HA
+    conf.setBoolean(DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED, false);

Review Comment:
   This still sounds like a bug though right? It is valid for a user to have ozone 1.1.0 (first layout version) and upgrade to Ozone 1.3.0 (schema v3) and turn on the schema v3 config when they do the upgrade. Schema v3 should still be disabled until the DN is finalized. When the DN finalizes, the rocksDB should be created and then schema v3 should work for all new containers.



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