You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "aryangupta1998 (via GitHub)" <gi...@apache.org> on 2023/09/14 07:47:38 UTC

[GitHub] [ozone] aryangupta1998 opened a new pull request, #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   ## What changes were proposed in this pull request?
   
   SCM deleting service should avoid sending delete transactions for under-replicated containers. It currently tracks all the transactions in memory until all the replicas receive the transaction. A lot of transactions for under-replicated containers could slow down the deletion as these will remain pending until the container reaches the desired replication. The service has a limit on pending transactions.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4368
   
   ## How was this patch tested?
   
   Tested Manually.
   


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > But isn't that the bug? We have sent 2 commands out, but are waiting for 3 responses. If you send 2, you should expect 2 back, not 3.
   
   Yes that's correct, this is old design could be because of we don't want to operate on rocksdb for each response as final goal is to delete from all replica. 
   
   > you are saying that the command would be resent to the new 3rd and then you will have 3 responses?
   
   Yes correct.
   
   
   
   > if we would be better ensuring we get the specific DN responses we care about, rather than always expecting `container.getReplicationConfig().getRequiredNodes()`
   
   This is done to ensure rocksdb transaction gets deleted only after all the required nodes has committed the transaction.


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   [transactionToDNsCommitMap](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L282) is kept in memory until response received from all the replicas. 
   I think, since it is just in memory when SCM restarts in under-replicated case it will again send request to all DN which may have already processed it, which is currently unnecessary.
   It may not look like it will slow down, Jira looks quite old so already some changes might have happened. Only thing map size will increase if more number of under-replicated containers are present and re-replication is getting delayed.


-- 
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] aryangupta1998 commented on a diff in pull request #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestDeleteWithInAdequateDN.java:
##########
@@ -296,14 +287,11 @@ public void testDeleteKeyWithSlowFollower() throws Exception {
         }
         return
             dnStateMachine.getCommandDispatcher()
-                .getDeleteBlocksCommandHandler().getInvocationCount() >= 1;
+                .getDeleteBlocksCommandHandler().getInvocationCount() >= 0;

Review Comment:
   Done



-- 
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] aryangupta1998 commented on pull request #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   @ChenSammi @sodonnel can you please take a 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] sodonnel commented on pull request #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > The check of 3 responses here is we want to make sure it can safely delete the transaction from the rocksdb table.
   
   But if there only 2 replicas, then this check is wrong. It should wait for 2 responses.
   
   I am also not sure that just skipping sending deletes to under replicated containers will fix this problem in all cases. Containers can be under-replicated due to mis-replication and have copies made, or the balancer could trigger a new copy to be made. If the goal is to avoid "missing" a delete, then what about this sort of scenario:
   
   * 3 replicas, but mis-replicated.
   * Send the deletes to the 3.
   * RM, at about the same time issues a replicate command for 1 just before its blocks are deleted. So replica 4 now exists.  * Block are deleted from the original 3. Will the logic notice the 4th and clean it up too or does it get left with the now orphan block?
   
   My point is that if you are under replicated at the time you process the blocks, then there are only 2 containers to remove the block from, and you should not wait for a 3rd response, then the under-replication check is not needed at all.
   
   My other point is that, I think, there are other opportunities for containers to end up missing block deletes with offline replicas, maintenance replicas, over replication, under replication and balancing.


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > > This is done to ensure rocksdb transaction gets deleted only after all the required nodes has committed the transaction.
   > 
   > But that is the bug - when you send to 2 nodes, you expect 2 responses not 3.
   
   The check of 3 responses here is we want to make sure it can safely delete the transaction from the rocksdb table.  If there are only 2 responses are received, then the delete transaction cannot be deleted from the rocksdb. It will be kept in rocksdb, and also in memory map transactionToDNsCommitMap and transactionToRetryCountMap.  Both the rocksdb and memory map will remove this transaction's record until the 3rd replica is reported, and the block deletion executed on this 3rd replica and acked to SCM. 
   
   If the 3rd replica disappears for a long time, and there are many such containers, then these two in memory map will hold objects which cannot be released for a long time.  This task aims to improve this situation. 
   
   Adding a timeout to the delete command is a good idea. But it's not related with this issue. We can support it in a new JIRA.
   
   
   


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -460,7 +463,13 @@ public DatanodeDeletedBlockTransactions getTransactions(
               txIDs.add(txn.getTxID());
             } else if (txn.getCount() > -1 && txn.getCount() <= maxRetry
                 && !containerManager.getContainer(id).isOpen()) {
-              getTransaction(txn, transactions, dnList);
+              Set<ContainerReplica> replicas = containerManager
+                  .getContainerReplicas(
+                      ContainerID.valueOf(txn.getContainerID()));
+              if (checkInadequateReplica(replicas, txn)) {

Review Comment:
   Under replication can be caused by more than just too few replicas. You should probably make a call to `replicationManager.getContainerReplicationHealth(...)` to get the replication state, as it incorporates the logic around under, over and mis-replication along with unhealthy replicas too.
   
   Also see my comments in the link below - what about balancing which can cause new replicas to get created and old replicas removed?
   
   https://issues.apache.org/jira/browse/HDDS-7728?focusedCommentId=17764748&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17764748



-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestDeleteWithInAdequateDN.java:
##########
@@ -296,14 +287,11 @@ public void testDeleteKeyWithSlowFollower() throws Exception {
         }
         return
             dnStateMachine.getCommandDispatcher()
-                .getDeleteBlocksCommandHandler().getInvocationCount() >= 1;
+                .getDeleteBlocksCommandHandler().getInvocationCount() >= 0;

Review Comment:
   This will always be true (>=0),  GenericTestUtils.waitFor(() {..} is not required. Command will not be invoked as one DN is down.



-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > It feels like we should be storing the DNs we sent the commands to, and checking off the replies against what we sent, rather than expecting replies from "all the current replica DNs", as the current replicas could have changed between when the command was issued and when the replies were received.
   
   Already SCM stores in memory to which DN command is already sent and which DN has already responded, to avoid retry on the successful responding DN.
   And this [memory](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L423)(in map) is kept until all the replica has responded, because after this only we can remove deleteTransactionID from rocksdb.
   If the current replica has changed after sending command, then in next retry it will send delete request to changed replica DN.
   
   > I think there should be a timeout, after which it discards the pending commands, and sends again to the current replica list and waits for a reply there. What do you think?
   
   In every retry it takes replica from [containerManager](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L271) and send out the delete request to DN. So always it is current replica.
   
   > What about, we have 3 replicas. We send 3 deletes, then one node goes down making it under replicated. We never get a reply from that node. Replication makes a 3rd copy before checking. This was not under-replicated at the start and the change here would not help I think.
   
   This behaviour remains same before and after this change. During retry it will send delete request to the new replica.
   
   
   ### Current behaviour:
   Assume replication factor:3
   If currently 2 replica present then send delete to 2 DNs. Entry remains in memory([transactionToDNsCommitMap](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L282)) as it requires 3 response.
   After certain try if another replica comes up, then send delete to the third replica DN.
   Remove entry from memory and then remove from rocksdb.
   
   ### Behaviour after this PR:
   Assume replication factor:3
   If currently 1-2 replica present then wait until it reaches replication factor number of replica before sending delete.
   Once replication factor satisfies send delete to all the 3 DNs.
   This avoids keeping track of [transactionToDNsCommitMap](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L282) for long time in most of the cases.
   
   


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > > This is done to ensure rocksdb transaction gets deleted only after all the required nodes has committed the transaction.
   > 
   > But that is the bug - when you send to 2 nodes, you expect 2 responses not 3.
   
   This PR fixes, it will send request to 3 nodes, and expecting response from 3 nodes.
   In case of under-replicated (like 2 or less node), It will not send the request at all and not expecting any response too.
   


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > This is done to ensure rocksdb transaction gets deleted only after all the required nodes has committed the transaction.
   
   But that is the bug - when you send to 2 nodes, you expect 2 responses not 3.


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


Re: [PR] HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers [ozone]

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


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > My other point is that, I think, there are other opportunities for containers to end up missing block deletes with offline replicas, maintenance replicas, over replication, under replication and balancing.
   
   @sodonnel , I think the original goal of this task when submitted by @lokeshj1703 , is to alleviate the SCM memory pressure posed by two in memory map **transactionToDNsCommitMap** and **transactionToRetryCountMap** due to this missing 3rd replica case causing the related block deletion transaction cannot be finished in a long time.  It doesn't aim to solve possible missing block deletes due to various reasons.  So what you mentioned is valid, but from another angel, and is beyond the current scope of this ticket.  
   
   Anyway, if it can solve extra problems in one patch without too much changes, then I think it's good to go.  One improvement we can have is leverage the replicationManager.getContainerReplicationHealth you mentioned, like this 
   
   ```
     private Boolean checkInadequateReplica(Set<ContainerReplica> replicas,
         ContainerInfo container) throws ContainerNotFoundException {
       ContainerHealthResult result =
           replicationManager.getContainerReplicationHealth(container, replicas);
       if (result.getHealthState() == ContainerHealthResult.HealthState.HEALTHY) {
         return true;
       }
       return false;
     }
   ```
   Then it will cover not only under-replicated containers, but also over-replicated and mis-replicated containers.  For other cases you mentioned, they can be addressed with follow up JIRAs and new patches. What do you think? BTW, you mentioned the maintenance replicas, how will it cause block not 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] sodonnel commented on pull request #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   I am not sure I understand the problem with sending deletes to under replicated containers. Surely the delete block management logic sends the commands to the replicas it has available and waits for confirmation from them it has succeeded?
   
   If you only have 2 replicas instead of 3, then the deleting service will just send out commands to the 2 replicas - how does under replication cause the deletes to be slow and potentially get stuck?


-- 
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 #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   OK - so say a container is under replicated. We send out 2 deletes to the 2 DNs. Then a 3rd replica appears. It did not get sent the delete, but now the logic expects that it needs 3 replies and it will never get that 3rd reply. In this case, could you argue that after some reasonable timeout, the delete should be sent again? If a DN receives a delete for a block is has not got, it will reply with an OK, right? So after a timeout we try again and it cleans up.
   
   What about, we have 3 replicas. We send 3 deletes, then one node goes down making it under replicated. We never get a reply from that node. Replication makes a 3rd copy before checking. This was not under-replicated at the start and the change here would not help I think.
   
   It feels like we should be storing the DNs we sent the commands to, and checking off the replies against what we sent, rather than expecting replies from "all the current replica DNs", as the current replicas could have changed between when the command was issued and when the replies were received.
   
   I think there should be a timeout, after which it discards the pending commands, and sends again to the current replica list and waits for a reply there. 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] sodonnel commented on pull request #5293: HDDS-4368. SCM should avoid sending delete transactions for under-replicated containers

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

   > If currently 2 replica present then send delete to 2 DNs. Entry remains in memory([transactionToDNsCommitMap](https://github.com/apache/ozone/blob/742734b00603e9ce9aea24a231a594c0cbc56604/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java#L282)) as it requires 3 response.
   
   But isn't that the bug? We have sent 2 commands out, but are waiting for 3 responses. If you send 2, you should expect 2 back, not 3.
   
   This is the code:
   
   ```
   final Set<ContainerReplica> replicas =
                 containerManager.getContainerReplicas(containerId);
             // The delete entry can be safely removed from the log if all the
             // corresponding nodes commit the txn. It is required to check that
             // the nodes returned in the pipeline match the replication factor.
      
            // Here - replicas.size() is the CM replica state NOW.
            // But dnsWithCommittedTxx, assuming all sent reply, is effectively replicas.size() in the past
            // So if the past was 2 as it was under replicated and now its 3, you are stuck. Although I think
            // you are saying that the command would be resent to the new 3rd and then you will have 3 responses?
            
             if (min(replicas.size(), dnsWithCommittedTxn.size()) 
                 >= container.getReplicationConfig().getRequiredNodes()) {
               List<UUID> containerDns = replicas.stream()
                   .map(ContainerReplica::getDatanodeDetails)
                   .map(DatanodeDetails::getUuid)
                   .collect(Collectors.toList());
               if (dnsWithCommittedTxn.containsAll(containerDns)) {
                 transactionToDNsCommitMap.remove(txID);
                 transactionToRetryCountMap.remove(txID);
                 if (LOG.isDebugEnabled()) {
                   LOG.debug("Purging txId={} from block deletion log", txID);
                 }
                 txIDsToBeDeleted.add(txID);
               }
             }
   ```
   
   I am just not sure if this change will fix this in all cases, and I wonder if we would be better ensuring we get the specific DN responses we care about, rather than always expecting `container.getReplicationConfig().getRequiredNodes()` if we didn't send out that many.


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