HDDS-12134. Implement Snapshot Cache lock for OM Bootstrap

[swamirishi]

What changes were proposed in this pull request?

Implementing a lock for this snapshot cache would prevent any newer threads from requesting a snapshot rocksdb handle from the snapshot cache. Thus any operation under this lock will have a consistent view of the entire snapshot. Creating a PR with an alternate approach which is lesser complex in implementation in comparison to #7745
The implementation uses are read lock whenever a handle to a rocksdb is requested. To acquire a complete cache lock write lock is taken and all rocksdb instances in the cache are closed. Till the write lock is held no thread would be able to open the rocksdb instance

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12134

How was this patch tested?

Adding unit tests.

[Copilot]

Pull Request Overview

This PR adds a snapshot cache lock mechanism to ensure consistent RocksDB handle views during OM bootstrap and related operations.

• Introduce IOzoneManagerLock and pass it into SnapshotCache to guard reads and writes.
• Refactor OzoneManagerLock/OmReadOnlyLock/IOzoneManagerLock to support flat‐resource write locks.
• Update snapshot‐related code, responses, services, and tests to use the new SNAPSHOT_DB_LOCK and verify lock behavior.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
TestSnapshotDiffManager.java	Inject OmReadOnlyLock into SnapshotCache in test setup
TestSnapshotCache.java	Add parameterized tests for read/write lock behavior in SnapshotCache
SnapshotCache.java	Add lock field, acquire/release read locks, and new write‐lock API (lock())
OMSnapshotPurgeResponse.java	Replace leveled SNAPSHOT_LOCK with flat SNAPSHOT_DB_LOCK
SstFilteringService.java	Update lock calls to use SNAPSHOT_DB_LOCK
OmSnapshotManager.java	Pass metadata manager’s lock into SnapshotCache constructor
TestOzoneManagerLock.java	Convert lock tests to parameterized and include flat‐resource scenarios
OzoneManagerLock.java	Refactor locking internals, add acquireResourceWriteLock/releaseResourceWriteLock, and new flat resource
OmReadOnlyLock.java	Implement no‐op flat‐resource write‐lock methods
IOzoneManagerLock.java	Extend interface to declare resource‐level write‐lock methods

Comments suppressed due to low confidence (1)

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java:289

• The code uses Arrays::stream but java.util.Arrays is not imported, causing a compile error. Add import java.util.Arrays;.

for (Resource resource : Stream.of(LeveledResource.values(), FlatResource.values())

[Copilot]

Using OmReadOnlyLock in this test will cause all get() calls to fail (lock always not acquired). Consider injecting a lock stub or the real OzoneManagerLock that grants read locks so the test can exercise cache behavior.

Suggested change

	SnapshotCache snapshotCache = new SnapshotCache(mockCacheLoader(), 10, omMetrics, 0,
	new OmReadOnlyLock());
	OzoneManagerLock mockLock = mock(OzoneManagerLock.class);
	when(mockLock.acquireReadLock(anyString())).thenReturn(true);
	SnapshotCache snapshotCache = new SnapshotCache(mockCacheLoader(), 10, omMetrics, 0,
	mockLock);

[Copilot]

[nitpick] The method name lock() is very generic and may be confused with the lock field. Consider renaming it to acquireSnapshotDbWriteLock() or similar for clarity.

[jojochuang]

looks like there's no test code that exercises this call so this annotation isn't needed and it can be made private.
Or we can add a test code for this method.

[swamirishi]

There are already testcases for testing snapshot eviction

[jojochuang]

if this method requires synchronized, this call is going to be expensive. should we make closing of OmSnapshot object asynchronous?

[swamirishi]

closing snapshot is asynchronous already.

[smengcl]

This one needs to be rebased

[swamirishi]

This one needs to be rebased

Done

[smengcl]

Thanks @swamirishi . lgtm apart from the comment nit. Pending CI.

[swamirishi]

Thank you for reviewing the PR @jojochuang & @smengcl

[swamirishi]

@sadanand48 We can use the snapshot cache api for the Bootstrap fixes. Please make the changes in #8477 or a follow up PR to make the bootstrap consistent.

[adoroszlai]

@swamirishi Have you noticed that this increased time for integration (flaky) by 25 minutes?

• before: ~50 minutes
• after: ~75 minutes

Most of the difference seems to be in TestSnapshotDeletingServiceIntegrationTest.

Before:

[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 168.2 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 31.12 s -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest

After:

[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 168.8 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[ERROR] Tests run: 2, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 449.1 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 157.8 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[ERROR] Tests run: 2, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 445.3 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[ERROR] Tests run: 3, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 155.1 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest
[ERROR] Tests run: 2, Failures: 0, Errors: 2, Skipped: 0, Time elapsed: 441.3 s <<< FAILURE! -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotDeletingServiceIntegrationTest

Created HDDS-13244.

[swamirishi]

TestSnapshotDeletingServiceIntegrationTest
Is this notice only after this got merged?