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/07/08 11:35:22 UTC
[GitHub] [ozone] JacksonYao287 opened a new pull request, #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
JacksonYao287 opened a new pull request, #3585:
URL: https://github.com/apache/ozone/pull/3585
## What changes were proposed in this pull request?
Cleanup RECOVERING container on DN restarts in containerReader
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-6978
--
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] JacksonYao287 commented on a diff in pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3585:
URL: https://github.com/apache/ozone/pull/3585#discussion_r917215316
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -207,6 +210,14 @@ public void verifyAndFixupContainerData(ContainerData containerData)
KeyValueContainerUtil.parseKVContainerData(kvContainerData, config);
KeyValueContainer kvContainer = new KeyValueContainer(kvContainerData,
config);
+ if (kvContainer.getContainerState() == RECOVERING) {
+ if (shouldDeleteRecovering) {
+ kvContainer.delete();
Review Comment:
yea, we should also delete the rocksDB entry of this container in merge rocksdb case
--
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] guihecheng commented on a diff in pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
guihecheng commented on code in PR #3585:
URL: https://github.com/apache/ozone/pull/3585#discussion_r917492709
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -207,6 +210,14 @@ public void verifyAndFixupContainerData(ContainerData containerData)
KeyValueContainerUtil.parseKVContainerData(kvContainerData, config);
KeyValueContainer kvContainer = new KeyValueContainer(kvContainerData,
config);
+ if (kvContainer.getContainerState() == RECOVERING) {
+ if (shouldDeleteRecovering) {
+ kvContainer.delete();
Review Comment:
I think `kvContainer.delete()` already handles deleting kv pairs in the rocksdb for both before and after rocksdb-merge.
--
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] JacksonYao287 commented on a diff in pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
JacksonYao287 commented on code in PR #3585:
URL: https://github.com/apache/ozone/pull/3585#discussion_r917215316
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -207,6 +210,14 @@ public void verifyAndFixupContainerData(ContainerData containerData)
KeyValueContainerUtil.parseKVContainerData(kvContainerData, config);
KeyValueContainer kvContainer = new KeyValueContainer(kvContainerData,
config);
+ if (kvContainer.getContainerState() == RECOVERING) {
+ if (shouldDeleteRecovering) {
+ kvContainer.delete();
Review Comment:
yea, we should also delete the rocksDB entry of this container in merge rocksdb case. we can create a separate patch for 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] umamaheswararao merged pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
umamaheswararao merged PR #3585:
URL: https://github.com/apache/ozone/pull/3585
--
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] umamaheswararao commented on pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on PR #3585:
URL: https://github.com/apache/ozone/pull/3585#issuecomment-1180003000
Thanks a lot, @JacksonYao287 for the contribution!
--
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] umamaheswararao commented on a diff in pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on code in PR #3585:
URL: https://github.com/apache/ozone/pull/3585#discussion_r917163949
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -207,6 +210,14 @@ public void verifyAndFixupContainerData(ContainerData containerData)
KeyValueContainerUtil.parseKVContainerData(kvContainerData, config);
KeyValueContainer kvContainer = new KeyValueContainer(kvContainerData,
config);
+ if (kvContainer.getContainerState() == RECOVERING) {
+ if (shouldDeleteRecovering) {
+ kvContainer.delete();
Review Comment:
Nit: Typo: delete --> Delete
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java:
##########
@@ -217,7 +217,7 @@ private List<Long> addBlocks(KeyValueContainer keyValueContainer,
@Test
public void testContainerReader() throws Exception {
Review Comment:
How about we simply add a test which should simply create Recovering container and restart the DN and check?
Refer: TestContainerCommandsEC for creating recovering containers.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java:
##########
@@ -207,6 +210,14 @@ public void verifyAndFixupContainerData(ContainerData containerData)
KeyValueContainerUtil.parseKVContainerData(kvContainerData, config);
KeyValueContainer kvContainer = new KeyValueContainer(kvContainerData,
config);
+ if (kvContainer.getContainerState() == RECOVERING) {
+ if (shouldDeleteRecovering) {
+ kvContainer.delete();
Review Comment:
Also I assume this delete will take care of cleaning the rocksDB entries as well.
CC: @guihecheng
--
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] guihecheng commented on pull request #3585: HDDS-6978. EC: Cleanup RECOVERING container on DN restarts
Posted by GitBox <gi...@apache.org>.
guihecheng commented on PR #3585:
URL: https://github.com/apache/ozone/pull/3585#issuecomment-1179884662
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.
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