HDDS-16143. Fix durable deleted-block summary undercount from unguarded flush()

[priyeshkaratha]

What changes were proposed in this pull request?

Fixes a permanent undercount of the durable DeletedBlocksTransactionSummary in SCM HA.

A transaction row and its updated summary are buffered as two separate writes in DeletedBlockLogStateManagerImpl. A flush() between them can persist the row without the summary. On a leader change, onBecomeLeader then reloads that stale summary and overwrites the correct in-memory counters. A reload can also land between the counter update and the DB write, with the same effect.

In this PR

• DeletedBlockLogStateManagerImpl takes the buffer lock around the row and summary writes in addTransactionsToDB and removeTransactionsFromDB, so a flush cannot split them. The lock is not held across the synchronous Ratis call, which avoids the flush-writer/applier lock cycle that stalled requests until the Ratis timeout.
• SCMDeletedBlockTransactionStatusManager updates the counters and takes the summary snapshot atomically under summaryLock. inFlightOps tracks add/remove calls whose DB update is unfinished, and the summary reload is skipped while any are in flight.
• New SCMHADBTransactionBuffer#flushAndRun flushes and runs an action under the write lock. SCMStateMachine uses it to flush and reload on becoming leader, and it terminates SCM if the flush fails.

What is the link to the Apache JIRA

HDDS-16143

How was this patch tested?

Added new testcases.

[priyeshkaratha]

Thanks @rich7420 for review. Addressed the comments in new commit.

[rich7420]

LGTM

[smengcl]

Hmm. The new locking can stall SCM requests until the Ratis timeout.

The add and remove paths hold the buffer read lock while waiting for a synchronous Ratis reply. If the flush monitor queues for the write lock before the applier acquires its read lock, the caller waits for the applier, the monitor waits for the caller, and the applier waits behind the monitor. Progress resumes only after the request times out—30 seconds by default.

Please avoid holding the buffer lock across the synchronous Ratis call while preserving protection against the summary reload race. A regression test should go through the HA proxy; the current tests call the implementation directly and miss this lock cycle.

[smengcl]

This call waits for Ratis through the HA proxy while holding the buffer read lock. A queued flush writer blocks the applier's read-lock acquisition, so the request cannot complete until its timeout releases the caller's lock. The remove path has the same issue.

Please coordinate summary updates and reloads without holding this lock across the synchronous Ratis call, and cover the interleaving through the HA proxy.

[priyeshkaratha]

Fixed. The caller no longer holds the buffer lock while it waits for Ratis. The buffer lock is taken only around the local buffer writes in DeletedBlockLogStateManagerImpl.addTransactionsToDB and removeTransactionsFromDB, which run on the apply side. The Ratis wait is outside any buffer lock, so a queued flush writer can't block the applier's read-lock acquisition. The reload-race protection comes from summaryLock plus inFlightOps and flushAndRun, not from holding the buffer lock.

[priyeshkaratha]

Hmm. The new locking can stall SCM requests until the Ratis timeout.

The add and remove paths hold the buffer read lock while waiting for a synchronous Ratis reply. If the flush monitor queues for the write lock before the applier acquires its read lock, the caller waits for the applier, the monitor waits for the caller, and the applier waits behind the monitor. Progress resumes only after the request times out—30 seconds by default.

Please avoid holding the buffer lock across the synchronous Ratis call while preserving protection against the summary reload race. A regression test should go through the HA proxy; the current tests call the implementation directly and miss this lock cycle.

Thanks @smengcl , the lock cycle was real: the caller waited for the applier, the flush monitor waited for the caller, and the applier waited behind the monitor until the 30s timeout. The caller no longer holds the buffer lock across the synchronous Ratis call, which breaks the cycle. The reload race is still guarded by summaryLock, inFlightOps and the atomic flush-and-reload on becoming leader. Details are in the thread replies above. I also removed an unused test hook and aligned the SCMHADBTransactionBufferStub.flushAndRun implementation with the real buffer.