HDDS-15342. Make RocksDB bottommost level compaction options configurable for background compaction by ptlrs · Pull Request #10419 · apache/ozone

ptlrs · 2026-06-03T01:09:03Z

What changes were proposed in this pull request?

RocksDB allows multiple configurations for configuration the compaction strategy for the bottom-most layer. These configurations impact the compaction times.

This PR makes the compaction strategy configurable.

Adds a config for setting the compaction strategy
Sets the default strategy to kSkip which is faster than kForce
Adds tests

What is the link to the Apache JIRA

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

How was this patch tested?

CI: https://github.com/ptlrs/ozone/actions/runs/26857346964

…igurability.

ashishkumar50 · 2026-06-08T05:06:18Z

  </property>
+  <property>
+    <name>ozone.om.compaction.service.bottommostlevelcompaction</name>
+    <value>0</value>


Can we use kSkip/kForce for better readability.

While I agree that a string option will be more readable, I have kept it this way because I want to use the native org.rocksdb.CompactRangeOptions.BottommostLevelCompaction#fromRocksId method.

That method converts an integer to compaction type. There is no method that RocksDB provides that converts a string to target compaction type.

The reason for keeping it this way is that I want to avoid writing and updating our custom method as and when RocksDB provides new compaction type options.

For example, the latest RocksDB already has a kForceOptimized option which our current version doesn't support. I want to avoid creating such manual mappings and validations in Ozone code.

In case we fail to provide a mapping for some future option, we will not be able to use that option even if RocksDB supports it.

As a compromise, the helper text provides the relevant information that is needed.

I want to avoid writing and updating our custom method as and when RocksDB provides new compaction type options

No custom method is needed if the config value is converted to enum BottommostLevelCompaction using Configuration.getEnum instead getInt.

I have updated the code to use getEnum

getEnum expects enum names with this change. Please update the value as well as the description.

sarvekshayr

Thanks @ptlrs for working on this.

…configuration logic.

…nfiguration and update its default type.

sarvekshayr · 2026-06-10T06:29:42Z

Failed acceptance (tools) CI - CompactOMDB defaults to kSkip which does not reduce the OM DB size in this test. #10428 should fix this test by passing the compaction type as kForce and be merged first.

Compact OM DB Column Family
    [Arguments]        ${column_family}
    Execute    ozone repair om compact --cf=${column_family} --service-id ${OM_SERVICE_ID} --node-id om1 --blc 2

ashishkumar50

@ptlrs Thanks for the update, LGTM.

ashishkumar50 · 2026-06-16T08:29:32Z

@ptlrs Can you please look at test failure, looks related.

ptlrs · 2026-06-16T16:54:36Z

@ashishkumar50 as @sarvekshayr had also observed, this PR depends on #10428 which fixes the failing Robot test.

Can we please merge that first? I may need to pull the changes in this PR after that.

…ocksDB-BottommostLevelCompaction-options-configurable # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/CompactDBUtil.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/CompactionService.java

… in OzoneManager.

ptlrs · 2026-06-17T18:42:22Z

Hi @ashishkumar50 @sarvekshayr, could you please take another look? I have merged the changes from master. The robot test should now pass.

adoroszlai · 2026-06-17T20:19:15Z

  </property>
+  <property>
+    <name>ozone.om.compaction.service.bottommostlevelcompaction</name>
+    <value>0</value>


I want to avoid writing and updating our custom method as and when RocksDB provides new compaction type options

No custom method is needed if the config value is converted to enum BottommostLevelCompaction using Configuration.getEnum instead getInt.

adoroszlai · 2026-06-17T20:26:13Z

    </description>
  </property>
+  <property>
+    <name>ozone.om.compaction.service.bottommostlevelcompaction</name>


Please use - as separator in last part of the property name.

Is this necessary? bottommostlevelcompaction is the original name as per org.rocksdb.CompactRangeOptions.BottommostLevelCompaction.

Most importantly, searching for bottommostlevelcompaction brings up all instances of it in both code and configuration in my IDE. Adding a separator would break that.

BottommostLevelCompaction uses CamelCase. The same with lower case needs some separator, e.g. - (kebab-case) or _ (snake_case) for readability. In config property names - is commonly used.

adoroszlai · 2026-06-17T20:29:08Z

+  @Test
+  void testCompactWithKSkip() {
+    assertDoesNotThrow(() ->
+        CompactDBUtil.compactTable(omMetadataManager, "keyTable",
+            ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip));
+  }
+
+  @Test
+  void testCompactWithKForce() {
+    assertDoesNotThrow(() ->
+        CompactDBUtil.compactTable(omMetadataManager, "keyTable",
+            ManagedCompactRangeOptions.BottommostLevelCompaction.kForce));
+  }
+
+  @Test
+  void testCompactWithKIfHaveCompactionFilter() {
+    assertDoesNotThrow(() ->
+        CompactDBUtil.compactTable(omMetadataManager, "keyTable",
+            ManagedCompactRangeOptions.BottommostLevelCompaction.kIfHaveCompactionFilter));
+  }


Please use @ParameterizedTest with @EnumSource.

adoroszlai · 2026-06-17T20:40:50Z

+  @Test
+  void testConfigValueMapsToCorrectEnum() {
+    assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip,
+        ManagedCompactRangeOptions.BottommostLevelCompaction.fromRocksId(0));
+    assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kIfHaveCompactionFilter,
+        ManagedCompactRangeOptions.BottommostLevelCompaction.fromRocksId(1));
+    assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kForce,
+        ManagedCompactRangeOptions.BottommostLevelCompaction.fromRocksId(2));
+  }
+
+  @Test
+  void testInvalidConfigValueReturnsNull() {
+    assertNull(ManagedCompactRangeOptions.BottommostLevelCompaction.fromRocksId(99));
+  }


These tests exercising RocksDB method fromRocksId would be unnecessary with the enum config approach.

github-actions · 2026-07-09T00:24:08Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

sarvekshayr · 2026-07-09T09:32:40Z

@ptlrs Could you please drive this PR to completion by addressing the minor comments given?

…ocksDB-BottommostLevelCompaction-options-configurable

…BottommostLevelCompaction configuration values.

sarvekshayr

Thanks for updating the patch @ptlrs.

Please look into the CI build failure.
Let's update compaction CLI to also use enum names instead of int - #10428
It can also be done in a follow-up PR.

sarvekshayr · 2026-07-14T10:37:44Z

+    <value>0</value>
+    <tag>OZONE, OM, PERFORMANCE</tag>
+    <description>
+      Bottommost level compaction type for compaction.
+      Invalid values will default to kSkip.
+      Valid values: 0 (kSkip), 1 (kIfHaveCompactionFilter), 2 (kForce), 3 (kForceOptimized).
+    </description>


getEnum expects enum names with this change. Please update the value as well as the description.

Suggested change

<value>0</value>

<tag>OZONE, OM, PERFORMANCE</tag>

<description>

Bottommost level compaction type for compaction.

Invalid values will default to kSkip.

Valid values: 0 (kSkip), 1 (kIfHaveCompactionFilter), 2 (kForce), 3 (kForceOptimized).

</description>

<value>kSkip</value>

<tag>OZONE, OM, PERFORMANCE</tag>

<description>

Bottommost level compaction type for compaction.

Invalid values will default to kSkip.

Valid values: kSkip, kIfHaveCompactionFilter, kForce, kForceOptimized.

</description>

sarvekshayr · 2026-07-14T10:39:19Z

+   * Bottommost level compaction type for manual compaction.
+   * Invalid values will default to kSkip.
+   * Valid values: 0 (kSkip), 1 (kIfHaveCompactionFilter), 2 (kForce), 3 (kForceOptimized).


Suggested change

* Bottommost level compaction type for manual compaction.

* Invalid values will default to kSkip.

* Valid values: 0 (kSkip), 1 (kIfHaveCompactionFilter), 2 (kForce), 3 (kForceOptimized).

* Bottommost level compaction type for background compaction service.

* Invalid values will default to kSkip.

* Valid values: kSkip, kIfHaveCompactionFilter, kForce, kForceOptimized.

sarvekshayr · 2026-07-14T10:42:50Z

+  @Test
+  public void testConfiguredCompactionLevelKForce() {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 2);


Suggested change

conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 2);

conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, "kForce");

sarvekshayr · 2026-07-14T10:43:41Z

+  @Test
+  public void testInvalidCompactionLevelFallsBackToDefault() {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 99);


Any invalid string can be used here -

Suggested change

conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 99);

conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, "abc");

sarvekshayr · 2026-07-14T10:47:12Z

    return compactableTables;
  }

+  @VisibleForTesting


@VisibleForTesting annotation is no longer used, see HDDS-12725.

Suggested change

@VisibleForTesting

ptlrs added 2 commits June 2, 2026 17:35

HDDS-15342. Make RocksDB BottommostLevelCompaction options configurable.

8c35336

HDDS-15342. Add unit tests for RocksDB BottommostLevelCompaction conf…

7281953

…igurability.

ashishkumar50 reviewed Jun 8, 2026

View reviewed changes

sarvekshayr reviewed Jun 8, 2026

View reviewed changes

HDDS-15342. Refactor to centralize RocksDB BottommostLevelCompaction …

059ac20

…configuration logic.

ptlrs mentioned this pull request Jun 9, 2026

HDDS-15466. Make RocksDB bottommost level compaction options configurable for CLI tools #10428

Merged

amaliujia reviewed Jun 9, 2026

View reviewed changes

Comment thread hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/CompactDBUtil.java Outdated

Comment thread hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java

HDDS-15342. Refactor to use getInt for BottommostLevelCompaction co…

422c85c

…nfiguration and update its default type.

ptlrs requested review from amaliujia, ashishkumar50 and sarvekshayr June 9, 2026 18:59

sarvekshayr marked this pull request as ready for review June 10, 2026 05:20

sarvekshayr reviewed Jun 10, 2026

View reviewed changes

Comment thread hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/CompactDBUtil.java

ashishkumar50 reviewed Jun 10, 2026

View reviewed changes

HDDS-15342. Update compaction configuration documentation

d216a87

ashishkumar50 approved these changes Jun 12, 2026

View reviewed changes

ptlrs added 2 commits June 17, 2026 10:23

HDDS-15342. Remove redundant BottommostLevelCompaction initialization…

d74e074

… in OzoneManager.

adoroszlai reviewed Jun 17, 2026

View reviewed changes

github-actions Bot added the stale label Jul 9, 2026

github-actions Bot removed the stale label Jul 10, 2026

Merge remote-tracking branch 'upstream/master' into HDDS-15342-Make-R…

a0d9c5a

…ocksDB-BottommostLevelCompaction-options-configurable

ptlrs requested a review from adoroszlai July 13, 2026 22:56

HDDS-15342. Refactor tests and improve handling of invalid or custom …

02e431e

…BottommostLevelCompaction configuration values.

sarvekshayr reviewed Jul 14, 2026

View reviewed changes

	conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 2);
	conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, "kForce");

Uh oh!

Conversation

ptlrs commented Jun 3, 2026

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ptlrs Jun 9, 2026 • 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!

sarvekshayr Jul 14, 2026 • 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!

sarvekshayr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sarvekshayr commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ashishkumar50 left a comment

Choose a reason for hiding this comment

Uh oh!

ashishkumar50 commented Jun 16, 2026

Uh oh!

ptlrs commented Jun 16, 2026

Uh oh!

ptlrs commented Jun 17, 2026

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!

adoroszlai Jul 14, 2026 • 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!

github-actions Bot commented Jul 9, 2026

Uh oh!

sarvekshayr commented Jul 9, 2026

Uh oh!

sarvekshayr 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!

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!

Reviewers

Assignees

ptlrs Jun 9, 2026 •

edited

Loading

sarvekshayr Jul 14, 2026 •

edited

Loading

sarvekshayr commented Jun 10, 2026 •

edited

Loading

adoroszlai Jul 14, 2026 •

edited

Loading

sarvekshayr left a comment •

edited

Loading