lokeshj1703 commented on a change in pull request #1805:
URL: https://github.com/apache/ozone/pull/1805#discussion_r561739335
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##########
@@ -139,19 +142,31 @@ public void incrementCount(List<Long> txIDs) throws
IOException {
}
continue;
}
- DeletedBlocksTransaction.Builder builder = block.toBuilder();
- int currentCount = block.getCount();
- if (currentCount > -1) {
- builder.setCount(++currentCount);
+
+ int currentCount =
+ transactionRetryCountMap.getOrDefault(txID, block.getCount());
+ if (currentCount >= 99) {
+ System.out.println();
}
Review comment:
Redundant change.
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##########
@@ -139,19 +142,31 @@ public void incrementCount(List<Long> txIDs) throws
IOException {
}
continue;
}
- DeletedBlocksTransaction.Builder builder = block.toBuilder();
- int currentCount = block.getCount();
- if (currentCount > -1) {
- builder.setCount(++currentCount);
+
+ int currentCount =
+ transactionRetryCountMap.getOrDefault(txID, block.getCount());
+ if (currentCount >= 99) {
+ System.out.println();
}
- // if the retry time exceeds the maxRetry value
- // then set the retry value to -1, stop retrying, admins can
- // analyze those blocks and purge them manually by SCMCli.
- if (currentCount > maxRetry) {
- builder.setCount(-1);
+ if (currentCount > -1) {
+ int nextCount = currentCount + 1;
+ DeletedBlocksTransaction.Builder builder = block.toBuilder();
+ if (nextCount > maxRetry) {
+ // if the retry time exceeds the maxRetry value
+ // then set the retry value to -1, stop retrying, admins can
+ // analyze those blocks and purge them manually by SCMCli.
+ builder.setCount(-1);
+ nextCount = -1;
+ scmMetadataStore.getDeletedBlocksTXTable().put(txID,
+ builder.build());
+ } else if ((nextCount / 100) - (currentCount / 100) > 0) {
Review comment:
It's difficult to understand the logic here. Can we replace it using %?
Perhaps `nextCount % 100 == 0 or 99`?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##########
@@ -387,6 +403,9 @@ public DatanodeDeletedBlockTransactions getTransactions(
}
}
purgeTransactions(txnsToBePurged);
+ for (DeletedBlocksTransaction trx : txnsToBePurged) {
+ transactionRetryCountMap.remove(trx.getTxID());
Review comment:
Unrelated to PR: Can we also remove from transactionToDNsCommitMap here
for better surity?
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##########
@@ -139,19 +142,31 @@ public void incrementCount(List<Long> txIDs) throws
IOException {
}
continue;
}
- DeletedBlocksTransaction.Builder builder = block.toBuilder();
- int currentCount = block.getCount();
- if (currentCount > -1) {
- builder.setCount(++currentCount);
+
+ int currentCount =
+ transactionRetryCountMap.getOrDefault(txID, block.getCount());
+ if (currentCount >= 99) {
+ System.out.println();
}
- // if the retry time exceeds the maxRetry value
- // then set the retry value to -1, stop retrying, admins can
- // analyze those blocks and purge them manually by SCMCli.
- if (currentCount > maxRetry) {
- builder.setCount(-1);
+ if (currentCount > -1) {
+ int nextCount = currentCount + 1;
+ DeletedBlocksTransaction.Builder builder = block.toBuilder();
+ if (nextCount > maxRetry) {
+ // if the retry time exceeds the maxRetry value
+ // then set the retry value to -1, stop retrying, admins can
+ // analyze those blocks and purge them manually by SCMCli.
+ builder.setCount(-1);
+ nextCount = -1;
+ scmMetadataStore.getDeletedBlocksTXTable().put(txID,
+ builder.build());
+ } else if ((nextCount / 100) - (currentCount / 100) > 0) {
+ // write retry count after every 100 retries into DB.
+ builder.setCount(nextCount);
+ scmMetadataStore.getDeletedBlocksTXTable().put(txID,
+ builder.build());
+ }
+ transactionRetryCountMap.put(txID, nextCount);
Review comment:
Can be moved inside else if
##########
File path:
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java
##########
@@ -139,19 +142,31 @@ public void incrementCount(List<Long> txIDs) throws
IOException {
}
continue;
}
- DeletedBlocksTransaction.Builder builder = block.toBuilder();
- int currentCount = block.getCount();
- if (currentCount > -1) {
- builder.setCount(++currentCount);
+
+ int currentCount =
+ transactionRetryCountMap.getOrDefault(txID, block.getCount());
+ if (currentCount >= 99) {
+ System.out.println();
}
- // if the retry time exceeds the maxRetry value
- // then set the retry value to -1, stop retrying, admins can
- // analyze those blocks and purge them manually by SCMCli.
- if (currentCount > maxRetry) {
- builder.setCount(-1);
+ if (currentCount > -1) {
+ int nextCount = currentCount + 1;
+ DeletedBlocksTransaction.Builder builder = block.toBuilder();
+ if (nextCount > maxRetry) {
+ // if the retry time exceeds the maxRetry value
+ // then set the retry value to -1, stop retrying, admins can
+ // analyze those blocks and purge them manually by SCMCli.
+ builder.setCount(-1);
+ nextCount = -1;
Review comment:
```suggestion
transactionRetryCountMap.remove(txID);
```
We can remove the entry from map here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
