HDDS-13026. KeyDeletingService should also delete RenameEntries #8447

swamirishi · 2025-05-14T00:48:34Z

What changes were proposed in this pull request?

Currently the entry from rename entries are never removed from snapshot renamed table which can lead to a bloat in the table size. Key deleting service should use ReclaimableRenameEntry to remove entries which don't even point to any object in the previous snapshot. This can be done as part of the snapshot deep cleaning and AOS garbage collection service

What is the link to the Apache JIRA

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

How was this patch tested?

Additional Unit tests

… each and every ratis request Change-Id: I5c29c572df9d2240b1d58fbc88826eb5ed8ad881

…shotInfo Change-Id: I16d2881af973799773335343debd856cb763f72b

Change-Id: I67d685aff12daaf98cf6a8eb5f6a5750c4cf6394

Change-Id: I4215cacc0f0486a7aa46b60971149483028bed38

Change-Id: I7a05f30d0a58538708e5acdbce36d836b6a5542e

Change-Id: I5a9e9dd3cc74e185b1f0745af7f2fee1026d125a # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java

Change-Id: I86c0adae571debf1e35e86660beb8c6a62d8b6fc

Change-Id: I403f238e8fd1178540da893f73a42aa2223572ad

Change-Id: Id3202dbe7a5a71c43526e80af34b18dd7b2ed66e

Change-Id: I17c1771c7630292fded0eebfbe6ee14f97798506

Change-Id: I03e67781351ed3a18666b32a12109141a5a4a34d

Change-Id: I1712f2dd4138a1f679eb4e4676b3d5ac41c80583

Change-Id: Id4a53d3eb8f23031ef517b3793dc73b067b11d62

Change-Id: I99896a697c523d6c174a06e66935a4be3b2bd541

jojochuang

huge code piece. need time to review.

jojochuang · 2025-05-16T21:34:18Z

...anager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java

@@ -140,6 +140,7 @@ private void updateSnapshotInfoAndCache(SnapshotInfo snapInfo, OmMetadataManager
      // current snapshot is deleted. We can potentially
      // reclaim more keys in the next snapshot.
      snapInfo.setDeepClean(false);
+      snapInfo.setDeepCleanedDeletedDir(false);


this is part of #8451?

jojochuang · 2025-05-16T23:45:03Z

...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java

+    /**
+     *
+     * @param currentSnapshotInfo if null, deleted directories in AOS should be processed.
+     * @param keyManager KeyManager of the underlying store.
+     */


/** * Scans the OM database for keys or rename entries that are eligible for deletion * in the given store (either the active store or a specific snapshot). * * It collects a batch of eligible keys and/or renamed entries up to the specified * remainNum limit, submits deletion requests to SCM, and, on successful confirmation, * removes the entries from the metadata store. The method also updates snapshot * properties related to reclaimed space and deep cleaning flags as required. * * @param currentSnapshotInfo If non-null, processes deleted keys for a specific snapshot; * if null, processes for the active object store. * @param keyManager The KeyManager instance for accessing keys in the underlying store. * @param remainNum The maximum number of entries to process in this invocation. * @return The number of entries remaining to be processed after this method runs. * @throws IOException If any error occurs during OM DB or SCM operations. */

jojochuang · 2025-05-16T23:47:19Z

...ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java

+     * @param currentSnapshotInfo if null, deleted directories in AOS should be processed.
+     * @param keyManager KeyManager of the underlying store.
+     */
+    private int processDeletedKeysForStore(SnapshotInfo currentSnapshotInfo, KeyManager keyManager,


this method appears to be extracted out of the below call(). But why is it much longer. Can it be broken up into smaller pieces.

hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto

...one-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java

jojochuang · 2025-05-16T23:57:20Z

also is there a test to verify that rename entries are removed after key deletion?

Change-Id: I76d096c4176bbf5508b6f75160c3524c1a04c5f0 # Conflicts: # hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/lock/OzoneManagerLock.java # hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDirectoryCleaningService.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java

Change-Id: Ibfcfe5d817ac6ac83f76abe3af08b5794bbb4b3e

Change-Id: I8137e7f06014951719dbf8736124f261193f2143 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/AbstractKeyDeletingService.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/KeyDeletingService.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestKeyDeletingService.java

Change-Id: I7af63cb80766fc83ec75ef9f7ad0ca8d299caa7a

Change-Id: Ib4c0199f50e5efb8f40b4e5c897d4b33acdf9d62

Change-Id: I67ca2fea367627a7a0ad97312ae450c74fabe95b

swamirishi · 2025-05-24T03:13:21Z

huge code piece. need time to review.

@jojochuang this pr size should have dropped significantly given that #8450 is merged

Change-Id: Ie9efcc5f15cee9d58cfcea7e373882c792cdd96b

Change-Id: I0f7cafce55e87884a0eb9fde3c5e15004b4cdb71

swamirishi · 2025-05-24T04:58:10Z

also is there a test to verify that rename entries are removed after key deletion?

Done adding a test case.

jojochuang

Looks good to me.

Optional, but we could consider adding a metric for removed renamed table entries. metadataManager.countRowsInTable(snapshotRenamedTable) should only be used in tests not used in production metrics.

.../ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java

Change-Id: I046604c6d5e7c049c99f6e65010a1a40dfbda965

Change-Id: Ifd2c97c3d932fb1335eed97b4535c0b6256add4b

smengcl · 2025-05-27T20:23:34Z

...ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/OMRequestTestUtils.java

+  public static String addRenamedEntryToTable(long trxnLogIndex, String volumeName, String bucketName, String key,
+                                            OMMetadataManager omMetadataManager) throws Exception {


nit: alignment is off

Suggested change

public static String addRenamedEntryToTable(long trxnLogIndex, String volumeName, String bucketName, String key,

OMMetadataManager omMetadataManager) throws Exception {

public static String addRenamedEntryToTable(long trxnLogIndex, String volumeName, String bucketName, String key,

OMMetadataManager omMetadataManager) throws Exception {

Change-Id: I16f7492a085ae56d6ce6402ee5fa2e102636d4bf

swamirishi · 2025-05-28T00:35:32Z

Thank you for reviewing the patch @jojochuang & @smengcl

swamirishi added 14 commits May 11, 2025 20:12

HDDS-13021. AbstractKeyDeletingService should have unique call id for…

8c626c6

… each and every ratis request Change-Id: I5c29c572df9d2240b1d58fbc88826eb5ed8ad881

HDDS-13022. Segragate Exclusive Dir Size and ExclusiveKeySize in Snap…

50b1a23

…shotInfo Change-Id: I16d2881af973799773335343debd856cb763f72b

HDDS-13022. Add Test OzoneSnapshot client code

685aac2

Change-Id: I67d685aff12daaf98cf6a8eb5f6a5750c4cf6394

HDDS-13022. Add hashcode for OzoneSnapshots

c29d3d8

Change-Id: I4215cacc0f0486a7aa46b60971149483028bed38

HDDS-13022. Add hashcode for OzoneSnapshots

49184f6

Change-Id: I7a05f30d0a58538708e5acdbce36d836b6a5542e

HDDS-13022. Address review comments

ea94417

Change-Id: I86c0adae571debf1e35e86660beb8c6a62d8b6fc

HDDS-13022. Address review comments

7bcbda6

Change-Id: I403f238e8fd1178540da893f73a42aa2223572ad

HDDS-13031. Remove Lock set bits for SNAPSHOT_GC_LOCK

b86ec7e

Change-Id: Id3202dbe7a5a71c43526e80af34b18dd7b2ed66e

HDDS-13026. KeyDeleting service should also delete RenameEntries

617cf00

Change-Id: I17c1771c7630292fded0eebfbe6ee14f97798506

Merge remote-tracking branch 'origin/HDDS-13031' into HEAD

33435cb

Change-Id: I03e67781351ed3a18666b32a12109141a5a4a34d

HDDS-13031. Fix Locking setbit issue

40b6a83

Change-Id: I1712f2dd4138a1f679eb4e4676b3d5ac41c80583

HDDS-13031. Fix checkstyle

c962507

Change-Id: Id4a53d3eb8f23031ef517b3793dc73b067b11d62

Merge remote-tracking branch 'origin/HDDS-13031' into HEAD

29b1c6d

Change-Id: I99896a697c523d6c174a06e66935a4be3b2bd541

swamirishi requested review from smengcl and jojochuang May 14, 2025 09:50

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

swamirishi requested a review from sadanand48 May 15, 2025 14:43

jojochuang reviewed May 16, 2025

View reviewed changes

swamirishi added 6 commits May 20, 2025 18:37

HDDS-13026. Fix checkstyle

4b84307

Change-Id: Ibfcfe5d817ac6ac83f76abe3af08b5794bbb4b3e

HDDS-13026. Merge with master

6116cba

Change-Id: I7af63cb80766fc83ec75ef9f7ad0ca8d299caa7a

HDDS-13026. Merge with master

3af0ac6

Change-Id: Ib4c0199f50e5efb8f40b4e5c897d4b33acdf9d62

HDDS-13026. Add OmKeyPurgeRequest changes

6249698

Change-Id: I67ca2fea367627a7a0ad97312ae450c74fabe95b

swamirishi added 2 commits May 24, 2025 00:03

HDDS-13026. Add test case

84dcb6b

Change-Id: Ie9efcc5f15cee9d58cfcea7e373882c792cdd96b

HDDS-13026. Add test case

ea8a3bc

Change-Id: I0f7cafce55e87884a0eb9fde3c5e15004b4cdb71

jojochuang reviewed May 26, 2025

View reviewed changes

.../ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyPurgeResponse.java Show resolved Hide resolved

swamirishi added 2 commits May 26, 2025 19:31

HDDS-13026. Add rename entries purged in metrics

fba3c21

Change-Id: I046604c6d5e7c049c99f6e65010a1a40dfbda965

HDDS-13026. Fix test

4f89ca6

Change-Id: Ifd2c97c3d932fb1335eed97b4535c0b6256add4b

swamirishi marked this pull request as ready for review May 27, 2025 17:43

jojochuang approved these changes May 27, 2025

View reviewed changes

smengcl changed the title ~~HDDS-13026. KeyDeleting service should also delete RenameEntries~~ HDDS-13026. KeyDeletingService should also delete RenameEntries May 27, 2025

smengcl reviewed May 27, 2025

View reviewed changes

HDDS-13026. Fix alignment

7786270

Change-Id: I16f7492a085ae56d6ce6402ee5fa2e102636d4bf

swamirishi merged commit fd3d70c into apache:master May 28, 2025
42 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

HDDS-13026. KeyDeletingService should also delete RenameEntries #8447

HDDS-13026. KeyDeletingService should also delete RenameEntries #8447

Uh oh!

swamirishi commented May 14, 2025

Uh oh!

jojochuang left a comment

Uh oh!

jojochuang May 16, 2025

Uh oh!

jojochuang May 16, 2025

Uh oh!

jojochuang May 16, 2025

Uh oh!

Uh oh!

Uh oh!

jojochuang commented May 16, 2025

Uh oh!

swamirishi commented May 24, 2025

Uh oh!

swamirishi commented May 24, 2025

Uh oh!

jojochuang left a comment

Uh oh!

Uh oh!

smengcl May 27, 2025

Uh oh!

swamirishi May 27, 2025

Uh oh!

swamirishi commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!

		public static String addRenamedEntryToTable(long trxnLogIndex, String volumeName, String bucketName, String key,
		OMMetadataManager omMetadataManager) throws Exception {

HDDS-13026. KeyDeletingService should also delete RenameEntries #8447

HDDS-13026. KeyDeletingService should also delete RenameEntries #8447

Uh oh!

Conversation

swamirishi commented May 14, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang May 16, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang May 16, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang May 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jojochuang commented May 16, 2025

Uh oh!

swamirishi commented May 24, 2025

Uh oh!

swamirishi commented May 24, 2025

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smengcl May 27, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi May 27, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi commented May 28, 2025

Uh oh!

Uh oh!

Uh oh!