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/04/05 18:24:53 UTC
[GitHub] [ozone] hanishakoneru opened a new pull request, #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
hanishakoneru opened a new pull request, #3276:
URL: https://github.com/apache/ozone/pull/3276
## What changes were proposed in this pull request?
Container BlockCount and UsedBytes have not been not reliable. [HDDS-5359](https://issues.apache.org/jira/browse/HDDS-5359) fixes the issues with how blockCount and usedBytes are updated. [HDDS-6234](https://issues.apache.org/jira/browse/HDDS-6234) provides a Container Inspector and Repair tool to fix existing containers with wrong blockCount and usedBytes values in container metadata.
Even after the fix in [HDDS-5359](https://issues.apache.org/jira/browse/HDDS-5359), usedBytes cannot be trusted to be an accurate representation of the actual number of bytes in the container. This is because usedBytes is updated in memory first when a chunk is written and then updated in DB during the putBlock call. Also, there could be orphaned chunks in the container which contribute to the usedBytes.
After [HDDS-5359](https://issues.apache.org/jira/browse/HDDS-5359),blockCount is reliable for new containers. So SCM should delete a container based on the blockCount = 0 and not check for usedBytes.
Also, when a DN receives a delete container command from SCM, it should double check that there are no valid blocks in the container before deleting it. This is an extra check on the DN side to avoid deleting a non-empty container.
## What is the link to the Apache JIRA
(https://issues.apache.org/jira/browse/HDDS-6555)
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove 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] errose28 merged pull request #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
Posted by GitBox <gi...@apache.org>.
errose28 merged PR #3276:
URL: https://github.com/apache/ozone/pull/3276
--
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] hanishakoneru commented on a diff in pull request #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on code in PR #3276:
URL: https://github.com/apache/ozone/pull/3276#discussion_r853328320
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java:
##########
@@ -818,6 +818,7 @@ public void deleteKey(OmKeyArgs args) throws IOException {
OMRequest omRequest = createOMRequest(Type.DeleteKey)
.setDeleteKeyRequest(req)
.build();
+ System.out.println("----- DeleteKey: " + args.getKeyName());
Review Comment:
Yes thanks Ethan. Removed 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] errose28 commented on pull request #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3276:
URL: https://github.com/apache/ozone/pull/3276#issuecomment-1106671162
Thanks for the fix @hanishakoneru. Merging since both datanode and SCM side tests have been added to address the comments.
--
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 a diff in pull request #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #3276:
URL: https://github.com/apache/ozone/pull/3276#discussion_r844035952
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java:
##########
@@ -1053,6 +1054,18 @@ private void deleteInternal(Container container, boolean force)
"Deletion of Open Container is not allowed.",
DELETE_ON_OPEN_CONTAINER);
}
+ // Safety check that the container is empty.
+ // If the container is not empty, it should not be deleted unless the
+ // container is beinf forcefully deleted (which happens when
+ // container is unhealthy or over-replicated).
+ if (container.getContainerData().getBlockCount() != 0) {
Review Comment:
It would be good to add a unit test on the DN side to validate that non-empty containers cannot be deleted, just incase someone makes changes in the future that breaks / removes this logic.
--
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 #3276: HDDS-6555. Container Deletion should not depend on usedBytes being zero
Posted by GitBox <gi...@apache.org>.
errose28 commented on code in PR #3276:
URL: https://github.com/apache/ozone/pull/3276#discussion_r849826365
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OzoneManagerProtocolClientSideTranslatorPB.java:
##########
@@ -818,6 +818,7 @@ public void deleteKey(OmKeyArgs args) throws IOException {
OMRequest omRequest = createOMRequest(Type.DeleteKey)
.setDeleteKeyRequest(req)
.build();
+ System.out.println("----- DeleteKey: " + args.getKeyName());
Review Comment:
I think this was a debug statement mistakenly left in.
--
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