aryangupta1998 commented on code in PR #10650:
URL: https://github.com/apache/ozone/pull/10650#discussion_r3541706302
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java:
##########
@@ -507,6 +507,11 @@ public void onMessage(
return;
}
+ if (!scmContext.isLeaderReady()) {
+ LOG.warn("SCM is not ready to commit transactions.");
+ return;
+ }
Review Comment:
```suggestion
if (!scmContext.isLeaderReady()) {
LOG.debug("Skip commit transactions since SCM leader is not ready yet.");
return;
}
```
onMessage() now logs at WARN for non-ready leader, this can spam logs during
leader warm-up because this path is heartbeat-driven, can we change it to debug?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -467,17 +465,37 @@ public void
addTransactions(ArrayList<DeletedBlocksTransaction> txList) throws I
incrDeletedBlocksSummary(tx);
}
}
- deletedBlockLogStateManager.addTransactionsToDB(txList, getSummary());
+ try {
+ deletedBlockLogStateManager.addTransactionsToDB(txList, getSummary());
+ } catch (IOException e) {
+ // Revert the in-memory changes if the DB update fails
+ for (DeletedBlocksTransaction tx: txList) {
+ if (tx.hasTotalBlockSize()) {
+ descDeletedBlocksSummary(tx);
+ LOG.warn("{} is decreased from summary due to DB update failure",
transactionToString(tx));
+ }
+ }
+ throw e;
+ }
return;
}
deletedBlockLogStateManager.addTransactionsToDB(txList);
}
+ private void incrDeletedBlocksSummary(TxBlockInfo txBlockInfo) {
+ totalTxCount.addAndGet(1);
+ totalBlockCount.addAndGet(txBlockInfo.getTotalBlockCount());
+ totalBlocksSize.addAndGet(txBlockInfo.getTotalBlockSize());
+
totalReplicatedBlocksSize.addAndGet(txBlockInfo.getTotalReplicatedBlockSize());
+ LOG.debug("Increase summary for {} to {}", txBlockInfo,
summaryToString(getSummary()));
Review Comment:
```suggestion
if (LOG.isDebugEnabled()) {
LOG.debug("Increase summary for {} to {}", txBlockInfo,
summaryToString(getSummary()));
}
```
Same we can do for,
```
descDeletedBlocksSummary(TxBlockInfo txBlockInfo)
descDeletedBlocksSummary(DeletedBlocksTransaction tx)
```
--
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]
