HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock #8446

swamirishi · 2025-05-13T23:31:16Z

What changes were proposed in this pull request?

SNAPSHOT_GC lock shouldn't interfere with locks taken by other Ratis requests. Implemented a generic Flat Lock resource which allows having resource which are independent of one another. This resource can be used to take a lock on any resource which wouldn't interfere with the existing Level ordering lock resource. We should move both SNAPSHOT_LOCK, SNAPSHOT_GC_LOCK into this.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing unit tests

Change-Id: Id3202dbe7a5a71c43526e80af34b18dd7b2ed66e

Change-Id: I1712f2dd4138a1f679eb4e4676b3d5ac41c80583

Change-Id: Id4a53d3eb8f23031ef517b3793dc73b067b11d62

Change-Id: Ie08fad6a07a66c75f6d823a64a7d278d1bfb54bd

Change-Id: Ibaa9a25cfc9126593b9ec3ec913c4a09e0846258

Change-Id: If61d109c573652c586b739baae49980749a4f9fa

Change-Id: I2b6ea2084bbb0320bf35608002faccf52b466b66

Change-Id: I8a11ed9a9cccb70b0cc05b295c0e5289fc859215

swamirishi · 2025-05-17T00:36:17Z

@smengcl Do you have any issues with the implementation?

Change-Id: If1f9d066e44fe3a324aff71278a0ca1858571166

adoroszlai · 2025-05-17T06:35:23Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java

-      String... resources) {
+                                        String... resources) {


Please change your IDE settings to avoid changing indent in method signatures like that.

done thanks for the tip!

Uncheck Method declaration parameters - Align when multiline like this:

@smengcl Thanks for the tip, I was using the Ozone checkstyle.xml, and it seems this is checked by default. Maybe we can fix it in checkstyle.xml instead.

yeah we should fix checkstyle.xml

#8494 please take a look at this pr

Change-Id: I533d58c79430f40b9169b04e24e27820456446ca

Change-Id: I2d9cfeb3f3b8432f109b8e59f0be8a18690d3799

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java

…to LeveledResource Change-Id: I04168bc2c9f30ffe81ec25f5261108494cd2f8f0

swamirishi · 2025-05-19T18:16:28Z

@smengcl Please check #8482

Change-Id: Idf969634141b07dbedfb0c2c4e0a13abb23b1e63 # Conflicts: # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java # hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java

Change-Id: Ice413ca799b8a834221d1e76312ef6e699e960e2 # Conflicts: # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/IOzoneManagerLock.java # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java # hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/MultiSnapshotLocks.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/filter/ReclaimableFilter.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/filter/AbstractReclaimableFilterTest.java

smengcl · 2025-05-20T18:20:11Z

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

+      lock.acquireWriteLock(otherResource, otherResourceName);
+      try {
+        lock.acquireWriteLock(flatResource, flatResourceName);


Let's add a @Timeout annotation to this case. In the negative case where flatResource is actually contentious with otherResource, it'd become a deadlock.

Timeout is set globally via config:

ozone/pom.xml

Line 2322 in 435fe7e

junit.jupiter.execution.timeout.default = 5m</configurationParameters>

Add annotation only if you need much shorter value.

no there would be no deadlock if threads use the bulk locks api that should only cause contention on resource and not deadlock.

patch updated

smengcl · 2025-05-20T20:03:38Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java

-    }
-    return currentLocks;
+    return resourcelockMap.values().stream().map(Pair::getValue)
+        .flatMap(i -> ((ResourceLockManager<? extends Resource>)i).getCurrentLockedResources())


nit

Suggested change

.flatMap(i -> ((ResourceLockManager<? extends Resource>)i).getCurrentLockedResources())

.flatMap(rlm -> ((ResourceLockManager<? extends Resource>) rlm).getCurrentLockedResources())

smengcl · 2025-05-20T20:22:40Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java

+
+    @Override
+    Stream<FlatResource> getCurrentLockedResources() {
+      return acquiredLocksMap.keySet().stream().filter(i -> acquiredLocksMap.get(i).get());


nit

Suggested change

return acquiredLocksMap.keySet().stream().filter(i -> acquiredLocksMap.get(i).get());

return acquiredLocksMap.keySet().stream().filter(flatResource -> acquiredLocksMap.get(flatResource).get());

smengcl

lgtm.

This looks to be a faithful refactoring, keeping most of the original behavior except for acquireMultiUserLock and releaseMultiUserLock and implementing FlatResource.

Change-Id: I2fe13d856c92437494cf8370418366573b81c815

swamirishi · 2025-05-20T20:56:41Z

Thanks for reviewing the PR @smengcl & @adoroszlai I will merge the PR once I get a green CI

…e#8446)

…239-container-reconciliation Commits: 80 commits 5e273a4 HDDS-12977. Fail build on dependency problems (apache#8574) 5081ba2 HDDS-13034. Refactor DirectoryDeletingService to use ReclaimableDirFilter and ReclaimableKeyFilter (apache#8546) e936e4d HDDS-12134. Implement Snapshot Cache lock for OM Bootstrap (apache#8474) 31d13de HDDS-13165. [Docs] Python client developer guide. (apache#8556) 9e6955e HDDS-13205. Bump common-custom-user-data-maven-extension to 2.0.3 (apache#8581) 750b629 HDDS-13203. Bump Bouncy Castle to 1.81 (apache#8580) ba5177e HDDS-13202. Bump build-helper-maven-plugin to 3.6.1 (apache#8579) 07ee5dd HDDS-13204. Bump awssdk to 2.31.59 (apache#8582) e1964f2 HDDS-13201. Bump jersey2 to 2.47 (apache#8578) 81295a5 HDDS-13013. [Snapshot] Add metrics and tests for snapshot operations. (apache#8436) b3d75ab HDDS-12976. Clean up unused dependencies (apache#8521) e0f08b2 HDDS-13179. rename-generated-config fails on re-compile without clean (apache#8569) f388317 HDDS-12554. Support callback on completed reconfiguration (apache#8391) c13a3fe HDDS-13154 Link more Grafana dashboard json files to the Observability user doc (apache#8533) 2a761f7 HDDS-11967. [Docs]DistCP Integration in Kerberized environment. (apache#8531) 81fc4c4 HDDS-12550. Use DatanodeID instead of UUID in NodeManager CommandQueue. (apache#8560) 2360af4 HDDS-13169. Intermittent failure in testSnapshotOperationsNotBlockedDuringCompaction (apache#8553) f19789d HDDS-13170. Reclaimable filter should always reclaim entries when buckets and volumes have already been deleted (apache#8551) 315ef20 HDDS-13175. Leftover reference to OM-specific trash implementation (apache#8563) 902e715 HDDS-13159. Refactor KeyManagerImpl for getting deleted subdirectories and deleted subFiles (apache#8538) 46a93d0 HDDS-12817. Addendum rename ecIndex to replicaIndex in chunkinfo output (apache#8552) 19b9b9c HDDS-13166. Set pipeline ID in BlockExistenceVerifier to avoid cached pipeline with different node (apache#8549) b3ff67c HDDS-13068. Validate Container Balancer move timeout and replication timeout configs (apache#8490) 7a7b9a8 HDDS-13139. Introduce bucket layout flag in freon rk command (apache#8539) 3c25e7d HDDS-12595. Add verifier for container replica states (apache#8422) 6d59220 HDDS-13104. Move auditparser acceptance test under debug (apache#8527) 8e8c432 HDDS-13071. Documentation for Container Replica Debugger Tool (apache#8485) 0e8c8d4 HDDS-13158. Bump junit to 5.13.0 (apache#8537) 8e552b4 HDDS-13157. Bump exec-maven-plugin to 3.5.1 (apache#8534) 168f690 HDDS-13155. Bump jline to 3.30.4 (apache#8535) cc1e4d1 HDDS-13156. Bump awssdk to 2.31.54 (apache#8536) 3bfb7af HDDS-13136. KeyDeleting Service should not run for already deep cleaned snapshots (apache#8525) 006e691 HDDS-12503. Compact snapshot DB before evicting a snapshot out of cache (apache#8141) 568b228 HDDS-13067. Container Balancer delete commands should not be sent with an expiration time in the past (apache#8491) 53673c5 HDDS-11244. OmPurgeDirectoriesRequest should clean up File and Directory tables of AOS for deleted snapshot directories (apache#8509) 07f4868 HDDS-13099. ozone admin datanode list ignores --json flag when --id filter is used (apache#8500) 08c0ab8 HDDS-13075. Fix default value in description of container placement policy configs (apache#8511) 58c87a8 HDDS-12177. Set runtime scope where missing (apache#8513) 10c470d HDDS-12817. Add EC block index in the ozone debug replicas chunk-info (apache#8515) 7027ab7 HDDS-13124. Respect config hdds.datanode.use.datanode.hostname when reading from datanode (apache#8518) b8b226c HDDS-12928. datanode min free space configuration (apache#8388) fd3d70c HDDS-13026. KeyDeletingService should also delete RenameEntries (apache#8447) 4c1c6cf HDDS-12714. Create acceptance test framework for debug and repair tools (apache#8510) fff80fc HDDS-13118. Remove duplicate mockito-core dependency from hdds-test-utils (apache#8508) 10d5555 HDDS-13115. Bump awssdk to 2.31.50 (apache#8505) 360d139 HDDS-13017. Fix warnings due to non-test scoped test dependencies (apache#8479) 1db1cca HDDS-13116. Bump jline to 3.30.3 (apache#8504) 322ca93 HDDS-13025. Refactor KeyDeletingService to use ReclaimableKeyFilter (apache#8450) 988b447 HDDS-5287. Document S3 ACL classes (apache#8501) 64bb29d HDDS-12777. Use module-specific name for generated config files (apache#8475) 54ed115 HDDS-9210. Update snapshot chain restore test to incorporate snapshot delete. (apache#8484) 87dfa5a HDDS-13014. Improve PrometheusMetricsSink#normalizeName performance (apache#8438) 7cdc865 HDDS-13100. ozone admin datanode list --json should output a newline at the end (apache#8499) 9cc4194 HDDS-13089. [snapshot] Add an integration test to verify snapshotted data can be read by S3 SDK client (apache#8495) cb9867b HDDS-13065. Refactor SnapshotCache to return AutoCloseSupplier instead of ReferenceCounted (apache#8473) a88ff71 HDDS-10979. Support STANDARD_IA S3 storage class to accept EC replication config (apache#8399) 6ec8f85 HDDS-13080. Improve delete metrics to show number of timeout DN command from SCM (apache#8497) 3bb8858 HDDS-12378. Change default hdds.scm.safemode.min.datanode to 3 (apache#8331) 0171bef HDDS-13073. Set pipeline ID in checksums verifier to avoid cached pipeline with different node (apache#8480) 5c7726a HDDS-11539. OzoneClientCache `@PreDestroy` is never called (apache#8493) a8ed19b HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock (apache#8446) e9e8b30 HDDS-12935. Support unsigned chunked upload with STREAMING-UNSIGNED-PAYLOAD-TRAILER (apache#8366) 7590268 HDDS-13079. Improve logging in DN for delete operation. (apache#8489) 435fe7e HDDS-12870. Fix listObjects corner cases (apache#8307) eb5dabd HDDS-12926. Remove *.tmp.* exclusion in DU (apache#8486) eeb98c7 HDDS-13030. Snapshot Purge should unset deep cleaning flag for next 2 snapshots in the chain (apache#8451) 6bf121e HDDS-13032. Support proper S3OwnerId representation (apache#8478) 5d1b43d HDDS-13076. Refactor OzoneManagerLock class to rename Resource class to LeveledResource (apache#8482) bafe6d9 HDDS-13064. [snapshot] Add test coverage for SnapshotUtils.isBlockLocationInfoSame() (apache#8476) 7035846 HDDS-13040. Add user doc highlighting the difference between Ozone ACL and S3 ACL. (apache#8457) 1825cdf HDDS-13049. Deprecate VolumeName & BucketName in OmKeyPurgeRequest and prevent Key version purge on Block Deletion Failure (apache#8463) 211c76c HDDS-13060. Change NodeManager.addDatanodeCommand(..) to use DatanodeID (apache#8471) f410238 HDDS-13061. Add test for key ACL operations without permission (apache#8472) d1a2f48 HDDS-13057. Increment block delete processed transaction counts regardless of log level (apache#8466) 0cc6fcc HDDS-13043. Replace != with assertNotEquals in TestSCMContainerPlacementRackAware (apache#8470) e1c779a HDDS-13051. Use DatanodeID in server-scm. (apache#8465) 35e1126 HDDS-13042. [snapshot] Add future proofing test cases for unsupported file system API (apache#8458) 619c05d HDDS-13008. Exclude same SST files when calculating full snapdiff (apache#8423) 21b49d3 HDDS-12965. Fix warnings about "used undeclared" dependencies (apache#8468) 8136119 HDDS-13048. Create new module for Recon integration tests (apache#8464) Conflicts: hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeManager.java

…rLock (apache#8446) (cherry picked from commit a8ed19b) Conflicts: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OmReadOnlyLock.java hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/lock/TestOzoneManagerLock.java Change-Id: I148c18c7cbc6e828e82837ca539b132850b0be7a

HDDS-13031. Remove Lock set bits for SNAPSHOT_GC_LOCK

b86ec7e

Change-Id: Id3202dbe7a5a71c43526e80af34b18dd7b2ed66e

swamirishi requested review from hemantk-12, jojochuang and smengcl May 13, 2025 23:31

ivandika3 added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label May 14, 2025

swamirishi changed the title ~~HDDS-13031. Remove Lock set bits for SNAPSHOT_GC_LOCK~~ HDDS-13031. Change Lock order for SNAPSHOT_GC_LOCK May 14, 2025

HDDS-13031. Fix Locking setbit issue

40b6a83

Change-Id: I1712f2dd4138a1f679eb4e4676b3d5ac41c80583

swamirishi marked this pull request as draft May 14, 2025 09:19

swamirishi changed the title ~~HDDS-13031. Change Lock order for SNAPSHOT_GC_LOCK~~ HDDS-13031. Remove Lock order for SNAPSHOT_GC_LOCK May 14, 2025

HDDS-13031. Fix checkstyle

c962507

Change-Id: Id4a53d3eb8f23031ef517b3793dc73b067b11d62

swamirishi marked this pull request as ready for review May 14, 2025 18:32

Merge remote-tracking branch 'apache/master' into HEAD

5dc9edc

Change-Id: Ie08fad6a07a66c75f6d823a64a7d278d1bfb54bd

swamirishi changed the title ~~HDDS-13031. Remove Lock order for SNAPSHOT_GC_LOCK~~ HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock May 16, 2025

swamirishi added 3 commits May 16, 2025 13:11

HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock

6f12aae

Change-Id: Ibaa9a25cfc9126593b9ec3ec913c4a09e0846258

Merge remote-tracking branch 'apache/master' into HEAD

c8ea2e6

Change-Id: If61d109c573652c586b739baae49980749a4f9fa

HDDS-13031. Fix pmd

588dab3

Change-Id: I2b6ea2084bbb0320bf35608002faccf52b466b66

swamirishi marked this pull request as draft May 16, 2025 17:44

HDDS-13031. Fix lock details code

2cad0f1

Change-Id: I8a11ed9a9cccb70b0cc05b295c0e5289fc859215

swamirishi marked this pull request as ready for review May 16, 2025 22:36

swamirishi requested a review from sadanand48 May 16, 2025 23:41

Merge remote-tracking branch 'apache/master' into HEAD

6dcf230

Change-Id: If1f9d066e44fe3a324aff71278a0ca1858571166

adoroszlai previously requested changes May 17, 2025

View reviewed changes

swamirishi added 2 commits May 17, 2025 08:02

HDDS-13031. Fix alignment

75c4764

Change-Id: I533d58c79430f40b9169b04e24e27820456446ca

Merge remote-tracking branch 'apache/master' into HEAD

41aeccb

Change-Id: I2d9cfeb3f3b8432f109b8e59f0be8a18690d3799

adoroszlai reviewed May 19, 2025

View reviewed changes

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java Outdated Show resolved Hide resolved

HDDS-13076. Refactor OzoneManagerLock class to rename Resource class …

4e76337

…to LeveledResource Change-Id: I04168bc2c9f30ffe81ec25f5261108494cd2f8f0

swamirishi added 2 commits May 19, 2025 18:15

smengcl reviewed May 20, 2025

View reviewed changes

smengcl approved these changes May 20, 2025

View reviewed changes

HDDS-13031. Address review comments

82400c0

Change-Id: I2fe13d856c92437494cf8370418366573b81c815

swamirishi merged commit a8ed19b into apache:master May 20, 2025
42 checks passed

Tejaskriya pushed a commit to Tejaskriya/ozone that referenced this pull request May 29, 2025

HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock (apach…

3d54e8c

…e#8446)

	.flatMap(i -> ((ResourceLockManager<? extends Resource>)i).getCurrentLockedResources())
	.flatMap(rlm -> ((ResourceLockManager<? extends Resource>) rlm).getCurrentLockedResources())

	return acquiredLocksMap.keySet().stream().filter(i -> acquiredLocksMap.get(i).get());
	return acquiredLocksMap.keySet().stream().filter(flatResource -> acquiredLocksMap.get(flatResource).get());

HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock #8446

HDDS-13031. Implement a Flat Lock resource in OzoneManagerLock #8446

Uh oh!

Conversation

swamirishi commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

swamirishi commented May 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swamirishi commented May 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swamirishi May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smengcl May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi commented May 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

swamirishi commented May 13, 2025 •

edited

Loading

swamirishi May 20, 2025 •

edited

Loading

smengcl May 20, 2025 •

edited

Loading