HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. #7365

sadanand48 · 2024-10-28T08:00:40Z

What changes were proposed in this pull request?

Currently we limit the number of deletion objects (directories) in each run of DirectoryDeletingService based on Ratis request limit i.e If Ratis request proto size exceeds the limit , DDS stops sending further dirs for purging in the same run which means the task limit per run is not honoured. This PR fixes this by batching the requests as per Ratis proto limit and sending mutiple purge requests in the same run. This way the task limit per run is honoured.

What is the link to the Apache JIRA

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

How was this patch tested?

Unit tests

…DirectoryDeletingService.

errose28 · 2024-10-28T20:26:15Z

Thanks for working on this @sadanand48. Are you planning to handle this for SCM block deletion and other OM services in another PR? In that case maybe we should file the PRs under sub-jiras of the original HDDS-11508 Jira for easier tracking.

sadanand48 · 2024-10-29T11:48:05Z

Are you planning to handle this for SCM block deletion and other OM services

Sure, will create sub tasks for these when the current one is fixed.

errose28 · 2024-10-29T16:57:55Z

Sounds good, I think that will help with reviews too. In that case, let's use a new Jira for this PR and resolve HDDS-11508 once all the subtasks and their PRs are complete.

adoroszlai · 2024-12-11T18:09:49Z

@sadanand48 HDDS-11605 introduced some conflicts, can you please update the PR?

sumitagrawl

@sadanand48 Thanks for working over this, have few minor comments...

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

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

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

sadanand48 · 2025-01-02T08:04:23Z

remove the limit case and use only ratis size.

@sumitagrawl @errose28 Could you please review this ? I have implemented the above suggestion and only used ratis size as the limit for sending a delete request.

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/DeleteKeysResult.java

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

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

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

sumitagrawl

@sadanand48 We can remove the configuration and unused default value and variables. Other logic LGTM.

sumitagrawl

LGTM +1

aryangupta1998

Thanks for the patch, @sadanand48, and @sumitagrawl for the review, LGTM!

adoroszlai

Sorry for the late review, but I just noticed this potential performance problem:

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java

Lines 608 to 612 in 24aab04

    
           CodecBuffer keyCodecBuffer = raw.getKey(); 
        
           final KEY key = keyCodec.fromCodecBuffer(keyCodecBuffer); 
        
           CodecBuffer valueCodecBuffer = raw.getValue(); 
        
           final VALUE value = valueCodec.fromCodecBuffer(valueCodecBuffer); 
        
           return Table.newKeyValue(key, value, keyCodecBuffer.getArray(), valueCodecBuffer.getArray());

This creates byte[] copy of raw key and value:

ozone/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java

Lines 357 to 365 in 24aab04

    
             /** 
        
              * @return a new array containing the readable bytes. 
        
              * @see #readableBytes() 
        
              */ 
        
             public byte[] getArray() { 
        
               final byte[] array = new byte[readableBytes()]; 
        
               buf.readBytes(array); 
        
               return array; 
        
             }

Raw key is never used, raw value is used only for getting serialized size:

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

Line 2105 in 24aab04

long objectSerializedSize = entry.getRawValue().length;

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

Line 2152 in 24aab04

long objectSerializedSize = entry.getRawValue().length;

We can get serialized size without allocating new arrays:

ozone/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java

Lines 339 to 342 in 24aab04

    
           /** @return the number of bytes can be read. */ 
        
           public int readableBytes() { 
        
             return buf.readableBytes(); 
        
           }

I think getRawKey and getRawValue should be replaced.

Note that the conversion affects all iterators, not just the deleting service.

sadanand48 · 2025-02-03T08:19:03Z

Sure thanks @adoroszlai , will open a jira to handle this and raise a PR.

adoroszlai · 2025-02-03T08:21:53Z

Thanks @sadanand48, I have created HDDS-12186, now assigned to you.

* master: (168 commits) HDDS-12112. Fix interval used for Chunk Read/Write Dashboard (apache#7724) HDDS-12212. Fix grammar in decommissioning and observability documentation (apache#7815) HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801) HDDS-12200. Fix grammar in OM HA, EC and Snapshot doc (apache#7806) HDDS-12202. OpsCreate and OpsAppend metrics not incremented (apache#7811) HDDS-12203. Initialize block length before skip (apache#7809) HDDS-12183. Reuse cluster across safe test classes (apache#7793) HDDS-11714. resetDeletedBlockRetryCount with --all may fail and can cause long db lock in large cluster. (apache#7665) HDDS-12186. (addendum) Avoid array allocation for table iterator (apache#7799) HDDS-12186. Avoid array allocation for table iterator. (apache#7797) HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. (apache#7365) HDDS-12073. Don't show Source Bucket and Volume if null in DU metadata (apache#7760) HDDS-12142. Save logs from build check (apache#7782) HDDS-12163. Reduce number of individual getCapacity/getAvailable/getUsedSpace calls (apache#7790) HDDS-12176. Trivial dependency cleanup.(apache#7787) HDDS-12181. Bump jline to 3.29.0 (apache#7789) HDDS-12165. Refactor VolumeInfoMetrics to use getCurrentUsage (apache#7784) HDDS-12085. Add manual refresh button for DU page (apache#7780) HDDS-12132. Parameterize testUpdateTransactionInfoTable for SCM (apache#7768) HDDS-11277. Remove dependency on hadoop-hdfs in Ozone client (apache#7781) ... Conflicts: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java hadoop-ozone/dist/src/main/smoketest/admincli/container.robot hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java

…ee-improvements * HDDS-10239-container-reconciliation: (168 commits) HDDS-12112. Fix interval used for Chunk Read/Write Dashboard (apache#7724) HDDS-12212. Fix grammar in decommissioning and observability documentation (apache#7815) HDDS-12195. Implement skip() in OzoneFSInputStream (apache#7801) HDDS-12200. Fix grammar in OM HA, EC and Snapshot doc (apache#7806) HDDS-12202. OpsCreate and OpsAppend metrics not incremented (apache#7811) HDDS-12203. Initialize block length before skip (apache#7809) HDDS-12183. Reuse cluster across safe test classes (apache#7793) HDDS-11714. resetDeletedBlockRetryCount with --all may fail and can cause long db lock in large cluster. (apache#7665) HDDS-12186. (addendum) Avoid array allocation for table iterator (apache#7799) HDDS-12186. Avoid array allocation for table iterator. (apache#7797) HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. (apache#7365) HDDS-12073. Don't show Source Bucket and Volume if null in DU metadata (apache#7760) HDDS-12142. Save logs from build check (apache#7782) HDDS-12163. Reduce number of individual getCapacity/getAvailable/getUsedSpace calls (apache#7790) HDDS-12176. Trivial dependency cleanup.(apache#7787) HDDS-12181. Bump jline to 3.29.0 (apache#7789) HDDS-12165. Refactor VolumeInfoMetrics to use getCurrentUsage (apache#7784) HDDS-12085. Add manual refresh button for DU page (apache#7780) HDDS-12132. Parameterize testUpdateTransactionInfoTable for SCM (apache#7768) HDDS-11277. Remove dependency on hadoop-hdfs in Ozone client (apache#7781) ...

…DirectoryDeletingService. (apache#7365)

swamirishi · 2025-12-03T18:42:51Z

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

    }

-    return directories;
+    processedSubDirs = processedSubDirs || (!iterator.hasNext());


@sadanand48 @sumitagrawl This is wrong if the last entry was skipped in line 2115 then iterator.hasNext() would return false. But the last entry was never deleted. Because of this directory would end up getting deleting without moving the last subFile/subDirectory entry from fileTable/DirectoryTable to deletedTable/deletedDirectoryTable

…st size for DirectoryDeletingService. (apache#7365) (cherry picked from commit 24aab04) Conflicts: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestDirectoryDeletingServiceWithFSO.java hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconInsightsForDeletedDirectories.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/DirectoryDeletingService.java hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestDirectoryDeletingService.java Change-Id: I2bed696e59fca178e4be69f428e6bb44ba31a2f8

…DirectoryDeletingService. (apache#7365) Backported from master commit 24aab04. Conflicts resolved: - AbstractKeyDeletingService.java: Kept HEAD import structure, excluded DeletingServiceMetrics - TestDirectoryDeletingService.java: Kept test from HDDS-11605 backport, simplified to work with buffer-based limits - Integration tests: Kept HEAD versions of cleanup methods - Table.java: Added missing Objects import - KeyManagerImpl.java: Removed getDeletedDirEntries method (not in interface) - SnapshotDeletingService.java: Updated to use new method signatures - TestSnapshotDeletingService.java: Removed OZONE_PATH_DELETING_LIMIT_PER_TASK config

HDDS-11508. Decouple delete batch limits from Ratis request size for …

1a6670e

…DirectoryDeletingService.

sadanand48 requested review from aryangupta1998 and sumitagrawl October 29, 2024 06:23

sadanand48 marked this pull request as draft November 15, 2024 08:40

Sadanand Shenoy added 8 commits November 15, 2024 14:19

compute size per OmDirectoryInfo and filter

aef33d2

fix checkstyle and findbugs

c7221bc

fix test failures

7fd889e

Merge branch 'master' into HDDS-11508

d9d8d0a

cleanup code

53db51f

fix tests

e5482a4

fix checkstyle

bdcc132

fix test

260ee6e

sadanand48 marked this pull request as ready for review December 6, 2024 08:23

sadanand48 requested review from sumitagrawl and removed request for sumitagrawl December 9, 2024 07:02

Sadanand Shenoy added 2 commits December 16, 2024 13:07

Merge branch 'master' into HDDS-11508

c564543

fix checkstyle

f06d754

sumitagrawl reviewed Dec 17, 2024

View reviewed changes

Sadanand Shenoy added 2 commits January 2, 2025 13:21

Only use ratis buf limit

19afd0f

Merge branch 'master' into HDDS-11508

4dd6237

sadanand48 marked this pull request as draft January 2, 2025 08:00

sumitagrawl reviewed Jan 2, 2025

View reviewed changes

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java Show resolved Hide resolved

sumitagrawl reviewed Jan 2, 2025

View reviewed changes

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/DeleteKeysResult.java Show resolved Hide resolved

Sadanand Shenoy added 2 commits January 7, 2025 11:45

address comments

945fc01

fix build

0af32cb

sadanand48 marked this pull request as ready for review January 7, 2025 10:30

sumitagrawl reviewed Jan 8, 2025

View reviewed changes

Sadanand Shenoy added 4 commits January 8, 2025 12:24

address comments

e9eeeef

fix tests

1aa6ea7

Merge branch 'master' into HDDS-11508

62b9671

fix findbugs

6febb6f

sumitagrawl reviewed Jan 19, 2025

View reviewed changes

...ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java Show resolved Hide resolved

sumitagrawl reviewed Jan 19, 2025

View reviewed changes

Sadanand Shenoy added 2 commits January 20, 2025 12:45

cleanup unused configs

14e6f01

remove from xml

c90e0c1

sumitagrawl approved these changes Jan 21, 2025

View reviewed changes

fix test

fac7f26

sadanand48 marked this pull request as draft January 24, 2025 05:49

Sadanand Shenoy added 3 commits January 24, 2025 12:17

fix test

ebbed26

remove redundant test

fc8d10c

Merge branch 'master' into HDDS-11508

ffbbbbc

sadanand48 marked this pull request as ready for review February 3, 2025 04:21

aryangupta1998 approved these changes Feb 3, 2025

View reviewed changes

aryangupta1998 merged commit 24aab04 into apache:master Feb 3, 2025
54 checks passed

adoroszlai reviewed Feb 3, 2025

View reviewed changes

sadanand48 mentioned this pull request Feb 3, 2025

HDDS-12186. Avoid array allocation for table iterator. #7797

Merged

nandakumar131 pushed a commit to nandakumar131/ozone that referenced this pull request Feb 10, 2025

HDDS-11508. Decouple delete batch limits from Ratis request size for …

0fa552f

…DirectoryDeletingService. (apache#7365)

swamirishi reviewed Dec 3, 2025

View reviewed changes

	CodecBuffer keyCodecBuffer = raw.getKey();
	final KEY key = keyCodec.fromCodecBuffer(keyCodecBuffer);
	CodecBuffer valueCodecBuffer = raw.getValue();
	final VALUE value = valueCodec.fromCodecBuffer(valueCodecBuffer);
	return Table.newKeyValue(key, value, keyCodecBuffer.getArray(), valueCodecBuffer.getArray());

	/**
	* @return a new array containing the readable bytes.
	* @see #readableBytes()
	*/
	public byte[] getArray() {
	final byte[] array = new byte[readableBytes()];
	buf.readBytes(array);
	return array;
	}

	/** @return the number of bytes can be read. */
	public int readableBytes() {
	return buf.readableBytes();
	}

HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. #7365

HDDS-11508. Decouple delete batch limits from Ratis request size for DirectoryDeletingService. #7365

Uh oh!

Conversation

sadanand48 commented Oct 28, 2024

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

errose28 commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sadanand48 commented Oct 29, 2024

Uh oh!

errose28 commented Oct 29, 2024

Uh oh!

adoroszlai commented Dec 11, 2024

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sadanand48 commented Jan 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

sumitagrawl left a comment

Choose a reason for hiding this comment

Uh oh!

aryangupta1998 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adoroszlai left a comment • 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!

sadanand48 commented Feb 3, 2025

Uh oh!

adoroszlai commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swamirishi Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

errose28 commented Oct 28, 2024 •

edited

Loading

adoroszlai left a comment •

edited

Loading

adoroszlai commented Feb 3, 2025 •

edited

Loading