nandakumar131 commented on code in PR #7532:
URL: https://github.com/apache/ozone/pull/7532#discussion_r1883435451
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -344,6 +346,9 @@ public DatanodeDeletedBlockTransactions getTransactions(
try (TableIterator<Long,
? extends Table.KeyValue<Long, DeletedBlocksTransaction>> iter =
deletedBlockLogStateManager.getReadOnlyIterator()) {
+ if (nextProcessTransactionId != -1) {
+ iter.seek(nextProcessTransactionId);
Review Comment:
There is a possibility that the `nextProcessTransactionId` is deleted from
the table. In that case comparing `nextProcessTransactionId` with
`keyValue.getKey()` will make no sense.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -386,6 +406,19 @@ public DatanodeDeletedBlockTransactions getTransactions(
LOG.warn("Container: {} was not found for the transaction: {}.",
id, txn);
txIDs.add(txn.getTxID());
}
+ if (!iter.hasNext()) {
+ iter.seekToFirst();
+ if (transactions.getBlocksDeleted() >= blockDeletionLimit) {
+ nextProcessTransactionId = -1;
+ }
+ }
+ }
+ if (iter.hasNext() && nextProcessTransactionId != -1) {
+ // Store this value so that next iteration starts from this element
instead of beginning
+ nextProcessTransactionId = iter.next().getKey();
+ } else {
+ // No more element in the table, start next iteration from the
beginning
+ nextProcessTransactionId = -1L;
Review Comment:
I can understand the idea here after spending some time going over the
logic, but this is not easy to follow.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -360,6 +366,17 @@ public DatanodeDeletedBlockTransactions getTransactions(
transactions.getBlocksDeleted() < blockDeletionLimit) {
Table.KeyValue<Long, DeletedBlocksTransaction> keyValue =
iter.next();
DeletedBlocksTransaction txn = keyValue.getValue();
+
+ if (nextProcessTransactionId == keyValue.getKey() && !firstTime) {
+ // Before already processed this transactionId in the current
iteration.
+ // Table is completely iterated once
+ nextProcessTransactionId = -1;
+ break;
Review Comment:
If the `nextProcessTransactionId` is removed form the table, we will not
break here. This will end up adding duplicate entries into 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: [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]
