smengcl opened a new pull request, #10995:
URL: https://github.com/apache/ozone/pull/10995
## What changes were proposed in this pull request?
This PR prevents an OM follower from deadlocking while installing a
checkpoint.
During checkpoint installation, `OzoneManagerStateMachine.pause()` holds the
state-machine monitor and calls
`OzoneManagerDoubleBuffer.stop()`, which waits for
`OMDoubleBufferFlushThread` to exit. After committing a batch,
the flush thread calls `updateLastAppliedTermIndex()`, which previously
required the same state-machine monitor:
```text
InstallSnapshotThread
holds OzoneManagerStateMachine monitor in pause()
-> OzoneManagerDoubleBuffer.stop()
-> daemon.join()
-> waits for OMDoubleBufferFlushThread
OMDoubleBufferFlushThread
-> updateLastAppliedTermIndex()
-> waits for OzoneManagerStateMachine monitor
```
This PR separates lifecycle synchronization from term/index synchronization:
- `pause()`, `unpause()`, and `reinitialize()` remain synchronized on the
state-machine instance.
- `notifyTermIndexUpdated()` and `updateLastAppliedTermIndex()` use a
dedicated `termIndexLock`.
- Restoring a persisted last-applied term/index also uses `termIndexLock`.
- `takeSnapshotImpl()` remains synchronized on the state-machine instance so
an in-progress snapshot finishes
before checkpoint pause returns and installation can replace the metadata
store.
- Within `takeSnapshotImpl()`, `termIndexLock` provides a consistent
applied/notified/skipped state to persist.
The double-buffer callback therefore no longer needs the lifecycle monitor
held by `pause()`.
### Why remove the two `synchronized` method modifiers?
The existing synchronization was intentional, but using the state-machine
monitor for both lifecycle and
term/index state created unnecessary coupling.
[HDDS-6685](https://issues.apache.org/jira/browse/HDDS-6685) introduced
synchronization on `pause()`,
`unpause()`, and `reinitialize()` to serialize checkpoint lifecycle changes.
Later,
[HDDS-10026](https://issues.apache.org/jira/browse/HDDS-10026) introduced
synchronized term/index methods when
the transaction maps were replaced with `lastApplied`, `lastNotified`, and
`lastSkipped` state.
Because a synchronized instance method locks `this`, both changes used the
same state-machine monitor. This
allowed checkpoint pause and the double-buffer callback to wait on each
other.
This PR does not remove mutual exclusion from the term/index methods. Only
`notifyTermIndexUpdated()` and
`updateLastAppliedTermIndex()` drop the method modifier; their bodies
synchronize on `termIndexLock` instead.
Keeping their method modifiers would continue acquiring the lifecycle
monitor and preserve the coupling
responsible for the deadlock.
`takeSnapshotImpl()` intentionally remains synchronized. HDDS-10026 used
this synchronization to serialize
snapshot persistence, and the original rationale was discussed in
[PR
#5891](https://github.com/apache/ozone/pull/5891#discussion_r1441390612). It
additionally acquires
`termIndexLock` to derive a consistent term/index value.
The resulting lock order is state-machine monitor -> `termIndexLock`. No
path that holds `termIndexLock`
attempts to acquire the state-machine monitor.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16155
## How was this patch tested?
- Added `testPauseWhileDoubleBufferUpdatesLastAppliedIndex` using a real
`OzoneManagerDoubleBuffer` and metadata
DB.
- The test times out on unpatched master at the pause/flush-callback cycle.
- The test passes with this patch.
- Ran six related OM Ratis and double-buffer test classes: 75 tests passed.
- Ran `TestOMRatisSnapshots#testInstallSnapshotWithClientWrite`: passed.
- Ran OM checkstyle: no violations.
--
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]
