You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ashishkumar50 (via GitHub)" <gi...@apache.org> on 2023/06/20 10:02:49 UTC

[GitHub] [ozone] ashishkumar50 opened a new pull request, #4937: HDDS-8838. Update default datanode check empty containter on disk to false

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

   ## What changes were proposed in this pull request?
   
   Currently we are checking both rocks db as well as disk to determine whether container is really empty or not. There are certain scenario where blocks remains on disk and causing container delete fail forever.
   More details is in the Jira.
   In this PR we are changing default value of hdds.datanode.check.empty.container.on-disk.on.delete to false, which will make container empty check just depends on rocksdb by default.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-8838
   
   ## How was this patch tested?
   
   Verified using new integration test and with existing 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] sodonnel commented on pull request #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on PR #4937:
URL: https://github.com/apache/ozone/pull/4937#issuecomment-1601328639

   All checks are green @errose28 are you happy to commit the latest revision?


-- 
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] ashishkumar50 commented on a diff in pull request #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #4937:
URL: https://github.com/apache/ozone/pull/4937#discussion_r1236250269


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java:
##########
@@ -89,10 +89,10 @@ public class DatanodeConfiguration {
 
   public static final String
       OZONE_DATANODE_CHECK_EMPTY_CONTAINER_ON_DISK_ON_DELETE =
-      "hdds.datanode.check.empty.container.delete";
+      "hdds.datanode.check.empty.container.on-disk.on.delete";

Review Comment:
   Updated config and description.



-- 
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 #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #4937:
URL: https://github.com/apache/ozone/pull/4937#discussion_r1235662875


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java:
##########
@@ -89,10 +89,10 @@ public class DatanodeConfiguration {
 
   public static final String
       OZONE_DATANODE_CHECK_EMPTY_CONTAINER_ON_DISK_ON_DELETE =
-      "hdds.datanode.check.empty.container.delete";
+      "hdds.datanode.check.empty.container.on-disk.on.delete";

Review Comment:
   Minor suggestion since technically the RocksDB is still being checked and that is on the disk too.
   ```suggestion
         "hdds.datanode.check.empty.container.dir.on.delete";
   ```



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java:
##########
@@ -89,10 +89,10 @@ public class DatanodeConfiguration {
 
   public static final String
       OZONE_DATANODE_CHECK_EMPTY_CONTAINER_ON_DISK_ON_DELETE =
-      "hdds.datanode.check.empty.container.delete";
+      "hdds.datanode.check.empty.container.on-disk.on.delete";

Review Comment:
   Also we should probably add an `@Config` field with a description to make this consistent with the other configs in this class.



-- 
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 #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #4937:
URL: https://github.com/apache/ozone/pull/4937


-- 
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] ashishkumar50 commented on pull request #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #4937:
URL: https://github.com/apache/ozone/pull/4937#issuecomment-1598484869

   @sodonnel @errose28, Please help to review, Thanks.


-- 
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] ashishkumar50 commented on pull request #4937: HDDS-8838. Update default datanode check empty containter on disk to false

Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on PR #4937:
URL: https://github.com/apache/ozone/pull/4937#issuecomment-1600034052

   > Thanks for the quick change @ashishkumar50. Functionally the change looks good. Could we maybe change the names of the two tests and add javadoc explaining that they are testing the true and false values of this config? It makes sense now but the distinction may not be clear to later readers.
   
   Thanks @errose28, I have updated javadoc and test name to have clear understanding.


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