HDDS-13520. Always retry failed BlockDeleteTransactions in SCM BlockDeletingSerice. #8926
Conversation
Can you add links in the Jira to the changes mentioned here? |
The reason for removing the skipping logic is that we will no longer get stuck on failed transactions. We now process the other transactions before retrying the failed ones again. That change was done in PR #7532 |
ashishkumar50
left a comment
ashishkumar50
left a comment
There was a problem hiding this comment.
@nandakumar131 Thanks for removing retry count dependency from SCM.
|
|
||
| @Override | ||
| public void execute(ScmClient client) throws IOException { | ||
| int count; |
There was a problem hiding this comment.
There are no-op for these two commands, we can remove code from client. It should not create any compatibility issues.
If we want to support these commands for older server then we should retain code in client as before.
There was a problem hiding this comment.
makes sense, will remove the command.
|
@nandakumar131 would you like to resolve the conflict. |
|
Thanks @ashishkumar50 for the ping. I will update the PR. |
ashishkumar50
left a comment
ashishkumar50
left a comment
There was a problem hiding this comment.
Thanks @nandakumar131, LGTM.
|
Thanks @ashishkumar50 for the review and merge. |
3 participants
What changes were proposed in this pull request?
Always retry failed BlockDeleteTransactions in SCM BlockDeletingSerice.
After the recent changes that was done in the block deletion code in SCM, we don't need to skip transactions that have exceeded the max retry count. We can keep retrying the transactions as we will not be stuck due to set of transactions that do not succeed.
We can keep the count for debugging purpose but we don't need to skip the transaction once it has reached certain number of retries.
This change will retain the count but will remove the logic which marks the transaction as failed and option to reset the count (this is no longer required).
What is the link to the Apache JIRA
HDDS-13520
How was this patch tested?
Unit test updated according to the new behaviour.
CI Run: https://github.com/nandakumar131/ozone/actions/runs/16875112559