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/08/15 11:51:37 UTC
[GitHub] [ozone] symious opened a new pull request, #3683: HDDS-7126. Clean deletedBlock records of deleted containers
symious opened a new pull request, #3683:
URL: https://github.com/apache/ozone/pull/3683
## What changes were proposed in this pull request?
By default DeletedBlockLog fetches 20,000 records per iteration and send the deleteBlock command to datanodes.
There is a chance that the db of deletedTable has a lot of records of deleted containers, and these records won't be cleaned, which leads to the idle iteration of the fetches, thus causing SCM failed to send commands to datanodes.
This ticket is to clean the records of deleted containers to recover SCM's delete block operation.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7126
## How was this patch tested?
unit 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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r978996752
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
Containers are deleted only if block count is zero. In my opinion you could also have an integration testcase with underreplicated containers & going ahead deleting these blocks. This particular issue can happen then.
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r978997790
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -425,7 +425,11 @@ public DatanodeDeletedBlockTransactions getTransactions(
DeletedBlocksTransaction txn = keyValue.getValue();
final ContainerID id = ContainerID.valueOf(txn.getContainerID());
try {
- if (txn.getCount() > -1 && txn.getCount() <= maxRetry
+ if (containerManager.getContainer(id).isDeleted()) {
+ LOG.warn("Container: " + id + " was not deleted for the " +
Review Comment:
There seems to be a typo in the log. I believe it should be was deleted instead.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
Containers are deleted only if block count is zero. In my opinion you could also have an integration testcase with underreplicated containers & going ahead deleting these blocks. This particular issue can happen then.
--
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] symious commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r983263874
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
Sorry for late reply, but I don't see the relationship between `commitTransactions` and the cleaning of deleted 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
[GitHub] [ozone] kerneltime merged pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime merged PR #3683:
URL: https://github.com/apache/ozone/pull/3683
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r980363438
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
I think a comment could be added in the commitTransactions function instead where the record is being removed.
--
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] swamirishi commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1406943114
@errose28 @symious Since it has been long, would it be better to rebase with master & run CI so that things don't fail?
--
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 #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1256570710
Thanks for the analysis @swamirishi. Coordinating between the deleted block log and replication manager will be challenging so I think removing DELETED containers from the deleted block log when they are encountered like this patch is doing should be a good fix. I will review this PR. @symious can you please resolve the merge conflict?
--
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 #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1233284357
@swamirishi will be looking at a way to reproduce this issue.
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r978997790
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -425,7 +425,11 @@ public DatanodeDeletedBlockTransactions getTransactions(
DeletedBlocksTransaction txn = keyValue.getValue();
final ContainerID id = ContainerID.valueOf(txn.getContainerID());
try {
- if (txn.getCount() > -1 && txn.getCount() <= maxRetry
+ if (containerManager.getContainer(id).isDeleted()) {
+ LOG.warn("Container: " + id + " was not deleted for the " +
Review Comment:
There seems to be a typo in the log. I believe it should be "was deleted" instead.
--
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 #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1217017760
Right, I believe the reason there is no removal for deleted containers from being processed for block deletes is that a container cannot be deleted until all blocks from all its replicas are deleted. This should imply that the deleted block log has no more entries for that container.
1. `DeletedBlockLogImpl#commitTransactions` checks that all replicas have processed a block deletion for a container before removing the entry.
2. `LegacyReplicationManager#deleteContainerReplicas` checks that all replicas have deleted all blocks.
Since the container cannot be deleted until all blocks have been deleted, the deleted block log/table should not have any entries for the container by the time it is deleted. Do you see a case where this could fail and there are lingering delete block entries for a container which has already been deleted?
--
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] symious commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
symious commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1257082911
@errose28 @swamirishi Thank you for the review. PR updated.
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r978990798
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -425,7 +425,11 @@ public DatanodeDeletedBlockTransactions getTransactions(
DeletedBlocksTransaction txn = keyValue.getValue();
final ContainerID id = ContainerID.valueOf(txn.getContainerID());
try {
- if (txn.getCount() > -1 && txn.getCount() <= maxRetry
+ if (containerManager.getContainer(id).isDeleted()) {
+ LOG.warn("Container: " + id + " was not deleted for the " +
Review Comment:
There seems to be a typo in the log. I believe it should be was deleted instead.
--
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] symious commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
symious commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1217928319
Haven't looked into why the inconsistence occurs, might it be related to the container report from Datanode?
Anyway, the issue is causing our Prod cluster stuck in deleting blocks, and the PR helps to resolve the issue.
--
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 #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1215952780
Hi @symious, my understanding of the change here is that containers that have been deleted will not need individual blocks deleted anymore. However, the requirement for a container to be deleted is that all replicas report zero blocks/keys. See `LegacyReplicationManager#deleteContainerReplicas`. This means all block deletes for that replica should have been processed and removed from the log. Is there still a situation where deleted containers can end up in the DeletedBlockLog?
--
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] swamirishi commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1256542562
@errose28 @symious
DeletedContainers can occur in this the list of deleteBlockTransactionsBlocks. Following is the flow when we try to delete blocks.Blocks
Blocks from DeleteBlockTransaction are picked up and Blocks are marked for deletion and a DeleteCommandStatus response. So based on this response the SCM tries to commit the transaction DeleteBlockTransaction. The particular record from the table is removed only if min(# Replicas Deleted, Total Number of Replicas)>=Replication Factor(In case of Under Replication this won't be removed).
When the blocks are deleted InMemoryBlockCounters are reduced in datanode side & these counters are sent along with the heartbeat. Hence on the SCM side LegacyReplicationManager checks if max(# of blocks) in closed container replica=0. If it is 0 the container is deleted.
Hence as the block deletion & actual block deletion are asynchronous this can cause the record to stay in the deleteBlockTransaction Table even if the containers have been deleted.
The above patch actually fixes this issue.
--
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] symious commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r979330568
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
@swamirishi Thank you for the analysis, I added some comments in the PR instead of adding an integrated test. I tried to add the test earlier, but then I found the test is too time-costly, if we have too many tests for corner cases, the whole CI would become huge and long, so maybe a comment is enough here. What do you think?
--
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] symious commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
symious commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1216015352
@errose28 Thanks for the review.
I see the container state is updated in `LegacyReplicatioNmanager#deleteContaienrReplicas`, but I didn't find the remove operation on records in DeletedTable, please correct me if I'm wrong.
--
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] kerneltime commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1418519550
Thank you @symious 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] errose28 commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
errose28 commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1229050605
Sorry for the late response here. It's good to hear that this patch resolves the observed issue, but we should really try to understand why deleted containers are ending up in the deleted block log to make sure we have the correct fix. I would like to investigate this but have not had the time. @symious if you have time perhaps you could look in to how this case occurs as well, which will help us validate this fix.
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by GitBox <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r978990798
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -425,7 +425,11 @@ public DatanodeDeletedBlockTransactions getTransactions(
DeletedBlocksTransaction txn = keyValue.getValue();
final ContainerID id = ContainerID.valueOf(txn.getContainerID());
try {
- if (txn.getCount() > -1 && txn.getCount() <= maxRetry
+ if (containerManager.getContainer(id).isDeleted()) {
+ LOG.warn("Container: " + id + " was not deleted for the " +
Review Comment:
There seems to be a typo in the log. I believe it should be was deleted instead.
--
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] swamirishi commented on a diff in pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #3683:
URL: https://github.com/apache/ozone/pull/3683#discussion_r1089305343
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -527,6 +533,22 @@ public void testDeletedBlockTransactions()
Assertions.assertEquals(2, blocks.size());
}
+ @Test
+ public void testDeletedBlockTransactionsOfDeletedContainer()
+ throws IOException, TimeoutException {
+ int txNum = 10;
+ List<DeletedBlocksTransaction> blocks;
+
+ // Creates {TXNum} TX in the log.
+ Map<Long, List<Long>> deletedBlocks = generateData(txNum,
Review Comment:
Sorry for holding this PR for pretty long. The comment here could be anywhere I suggested in the commitTransaction function as there is already a comment around the same but not covering under replication part. The comment in this class should also be fine.
--
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] swamirishi commented on pull request #3683: HDDS-7126. Clean deletedBlock records of deleted containers
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #3683:
URL: https://github.com/apache/ozone/pull/3683#issuecomment-1406940315
LGTM
--
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