sodonnel commented on PR #3799:
URL: https://github.com/apache/ozone/pull/3799#issuecomment-1271414316
I think this change makes sense and I think the change you made to the unit
test validates the first scenario:
> but the DB value won't be changed until exceeding the max retry times
I'm not sure any tests catch the second scenario though:
> Reset retry count didn't reset the value of transactionToRetryCountMap.
Could you add or modify a test to correctly validate that the reset call
correctly resets the transactionToRetryCountMap? Perhaps by incrementing
failed, resetting, and incrementing again to some other value?
TestResetCount seems to check that the DB is cleared when reset is called,
but it doesn't increment again, which think would cause the bug, as the map
would hold the old value and increment it incorrectly. This is the
TestRestCount method:
```
// Reset the retry count, these transactions should be accessible.
resetCount(txIDs);
blocks = getAllTransactions();
for (DeletedBlocksTransaction block : blocks) {
Assertions.assertEquals(0, block.getCount());
}
Assertions.assertEquals(30 * THREE, blocks.size());
```
I think we need something to increment before this part which should
increment to "1" but before this fix it will increment to whatever was stored
in the map.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
