smengcl commented on code in PR #11187:
URL: https://github.com/apache/ozone/pull/11187#discussion_r4300090285
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -524,27 +536,36 @@ public void removeTransactions(ArrayList<Long> txIDs)
throws IOException {
}
if
(VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.STORAGE_SPACE_DISTRIBUTION)
&&
!disableDataDistributionForTest) {
- List<TxBlockInfo> removedTxBlockInfos = new ArrayList<>();
- for (Long txID: txIDs) {
- TxBlockInfo txBlockInfo = txSizeMap.remove(txID);
- if (txBlockInfo != null) {
- descDeletedBlocksSummary(txBlockInfo);
- removedTxBlockInfos.add(txBlockInfo);
- }
- }
+ inFlightOps.incrementAndGet();
try {
- deletedBlockLogStateManager.removeTransactionsFromDB(txIDs,
getSummary());
- } catch (IOException e) {
- if (isRatisTimeout(e)) {
- throw e;
+ List<TxBlockInfo> removedTxBlockInfos = new ArrayList<>();
+ DeletedBlocksTransactionSummary summary;
+ synchronized (summaryLock) {
+ for (Long txID: txIDs) {
+ TxBlockInfo txBlockInfo = txSizeMap.remove(txID);
+ if (txBlockInfo != null) {
+ descDeletedBlocksSummary(txBlockInfo);
+ removedTxBlockInfos.add(txBlockInfo);
+ }
+ }
+ summary = getSummary();
}
- // Revert the in-memory changes if the DB update fails
- for (TxBlockInfo txBlockInfo : removedTxBlockInfos) {
- txSizeMap.put(txBlockInfo.getTxId(), txBlockInfo);
- rollbackDeletedBlocksSummary(txBlockInfo);
- LOG.warn("{} is added back to txSizeMap and increased to summary due
to DB update failure", txBlockInfo);
+ try {
+ deletedBlockLogStateManager.removeTransactionsFromDB(txIDs, summary);
+ } catch (IOException e) {
+ if (isRatisTimeout(e)) {
+ throw e;
Review Comment:
If a leader transition overlaps a timed-out Ratis add/remove, the reload is
skipped because inFlightOps is still nonzero. The timeout path then retains the
in-memory change and only decrements inFlightOps; if the request never commits,
no later reload restores the durable summary. A subsequent successful operation
can persist this phantom count.
Please defer or repeat the reload when the in-flight operation reaches a
known outcome, or otherwise reconcile the timeout before retaining the
in-memory summary.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -737,13 +758,21 @@ public DeletedBlocksTransactionSummary
getTransactionSummary() {
private void initDataDistributionData() throws IOException {
DeletedBlocksTransactionSummary newSummary = loadDeletedBlocksSummary();
if (newSummary != null) {
- DeletedBlocksTransactionSummary currentSummary = getSummary();
- totalTxCount.set(newSummary.getTotalTransactionCount());
- totalBlockCount.set(newSummary.getTotalBlockCount());
- totalBlocksSize.set(newSummary.getTotalBlockSize());
- totalReplicatedBlocksSize.set(newSummary.getTotalBlockReplicatedSize());
- if (!isSummaryEqual(currentSummary, newSummary)) {
- LOG.info("Old summary {} is replaced.",
summaryToString(currentSummary));
+ synchronized (summaryLock) {
+ if (inFlightOps.get() > 0) {
Review Comment:
This reload check happens after onBecomeLeader() clears txSizeMap at line
430. If removeTransactions() has incremented inFlightOps but has not acquired
summaryLock yet, that clear removes the TxBlockInfo before line 545, so the
remove does not decrement the counters. This branch then skips the durable
reload, and the unchanged summary is persisted for a transaction that was
deleted.
Please coordinate txSizeMap.clear() with the in-flight remove operation, or
defer the leader reset until the operation has completed, and add a regression
test for this ordering.
--
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]
