HDDS-16145. Fix race between DeletedBlockLogStateManager transaction removal and SCM HA buffer flush - #11188
HDDS-16145. Fix race between DeletedBlockLogStateManager transaction removal and SCM HA buffer flush#11188priyeshkaratha wants to merge 1 commit into
Conversation
…removal and SCM HA buffer flush
| // (checkpoint download or leader transfer) cannot land between marking these txIDs as hidden and their | ||
| // removal being durably flushed. Otherwise onFlush() would reset deletingTxIDs while the row is still | ||
| // present, re-exposing it to the deletion scanner and causing the summary to be double-decremented. | ||
| transactionBuffer.lock(); |
There was a problem hiding this comment.
This can still combine two different flush epochs. deletedTable.iterator() is initialized before snapshotDeletingTxIDs; RDBTable.iterator() creates the underlying RocksDB iterator at that point. If flush() runs between those two initializers, the iterator retains the deleted row while onFlush() replaces deletingTxIDs with a new empty set, so the row is returned again.
I reproduced this by pausing after iterator creation, flushing the buffered delete, then resuming:
[ERROR] A durably deleted transaction was re-exposed
expected: <false> but was: <true>
Reversing the two initializers made the same test pass. Please capture deletingTxIDs before creating the iterator, or hold the buffer read lock while capturing both, and cover this interleaving in the regression test.
| transactionBuffer.updateLatestTrxInfo(TRX_INFO_T4); | ||
| transactionBuffer.flush(); | ||
|
|
||
| transactionBuffer.lock(); |
There was a problem hiding this comment.
nit: please release this lock in a finally block. If waitFor or the assertion fails before unlock(), the non-daemon flusher remains parked on the write lock until the test fork terminates.
|
@priyeshkaratha thanks for your patch! |
What changes were proposed in this pull request?
DeletedBlockLogStateManagerImpl#removeTransactionsFromDBmarks a batch of transaction IDs as hidden (deletingTxIDs), removes each one from the SCM HA transaction buffer, and writes an updated summary — as threeseparate calls, each independently protected only for its own duration by
SCMHADBTransactionBuffer's internal read lock.A concurrent
flush()(holding the buffer's write lock, e.g. triggered by the periodic flush monitor, a Ratis checkpoint download, or a leader transfer) can land in the gap between these calls. If it lands after atxID has been marked hidden but before it has actually been removed from the buffer,
flush()callsonFlush(), which resetsdeletingTxIDs— while the transaction's row is still durably present in the deleted-blocks table. The row is then re-exposed togetReadOnlyIterator()(the deletion scanner) even though its removal is still in progress, which can cause the transaction to be processed again and its summary count to be double-decremented.This PR adds
lock()/unlock()toSCMHADBTransactionBuffer(backed by the buffer's existingReentrantReadWriteLockread lock) and hasremoveTransactionsFromDBacquire it across the entire mark-remove-summary sequence. Since
flush()takes the write lock, it cannot proceed until the whole sequence has released the read lock, so it can only observe the sequence fully completed or not yet started — never midway through.What is the link to the Apache JIRA
HDDS-16145
How was this patch tested?
Added testcases