-
Notifications
You must be signed in to change notification settings - Fork 616
HDDS-15342. Make RocksDB bottommost level compaction options configurable for background compaction #10419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
HDDS-15342. Make RocksDB bottommost level compaction options configurable for background compaction #10419
Changes from all commits
8c35336
7281953
059ac20
422c85c
d216a87
c8e168c
d74e074
a0d9c5a
02e431e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1591,6 +1591,16 @@ | |||||||||||||||||||||||||||||
| If this is empty, no column families are compacted. | ||||||||||||||||||||||||||||||
| </description> | ||||||||||||||||||||||||||||||
| </property> | ||||||||||||||||||||||||||||||
| <property> | ||||||||||||||||||||||||||||||
| <name>ozone.om.compaction.service.bottommostlevelcompaction</name> | ||||||||||||||||||||||||||||||
| <value>0</value> | ||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While I agree that a string option will be more readable, I have kept it this way because I want to use the native 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 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No custom method is needed if the config value is converted to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have updated the code to use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||||||||||||
| <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> | ||||||||||||||||||||||||||||||
|
Comment on lines
+1596
to
+1602
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||
| </property> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| <property> | ||||||||||||||||||||||||||||||
| <name>ozone.om.snapshot.compact.non.snapshot.diff.tables</name> | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |||||||||||||
| import java.util.concurrent.TimeUnit; | ||||||||||||||
| import org.apache.hadoop.hdds.client.ReplicationFactor; | ||||||||||||||
| import org.apache.hadoop.hdds.client.ReplicationType; | ||||||||||||||
| import org.apache.hadoop.hdds.utils.db.managed.ManagedCompactRangeOptions; | ||||||||||||||
| import org.apache.hadoop.ozone.om.helpers.BucketLayout; | ||||||||||||||
| import org.apache.ratis.util.TimeDuration; | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -681,6 +682,18 @@ public final class OMConfigKeys { | |||||||||||||
| public static final String OZONE_OM_COMPACTION_SERVICE_COLUMNFAMILIES_DEFAULT = | ||||||||||||||
| "keyTable,fileTable,directoryTable,deletedTable,deletedDirectoryTable,multipartInfoTable,multipartPartsTable"; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Bottommost level compaction type for manual compaction. | ||||||||||||||
| * Invalid values will default to kSkip. | ||||||||||||||
| * Valid values: 0 (kSkip), 1 (kIfHaveCompactionFilter), 2 (kForce), 3 (kForceOptimized). | ||||||||||||||
|
Comment on lines
+686
to
+688
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| * Refer to {@code org.rocksdb.CompactRangeOptions.BottommostLevelCompaction}. | ||||||||||||||
| */ | ||||||||||||||
| public static final String OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION = | ||||||||||||||
|
ptlrs marked this conversation as resolved.
|
||||||||||||||
| "ozone.om.compaction.service.bottommostlevelcompaction"; | ||||||||||||||
| public static final ManagedCompactRangeOptions.BottommostLevelCompaction | ||||||||||||||
| OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION_DEFAULT = | ||||||||||||||
| ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip; | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Configuration to enable/disable non-snapshot diff table compaction when snapshots are evicted from cache. | ||||||||||||||
| */ | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -54,6 +54,7 @@ public class CompactionService extends BackgroundService { | |||
| private final AtomicBoolean suspended; | ||||
| // list of tables that can be compacted | ||||
| private final List<String> compactableTables; | ||||
| private final ManagedCompactRangeOptions.BottommostLevelCompaction bottommostLevelCompaction; | ||||
|
|
||||
| public CompactionService(OzoneManager ozoneManager, TimeUnit unit, long interval, long timeout, | ||||
| List<String> tables) { | ||||
|
|
@@ -66,6 +67,7 @@ public CompactionService(OzoneManager ozoneManager, TimeUnit unit, long interval | |||
| this.numCompactions = new AtomicLong(0); | ||||
| this.suspended = new AtomicBoolean(false); | ||||
| this.compactableTables = validateTables(tables); | ||||
| this.bottommostLevelCompaction = CompactDBUtil.getBottommostLevelCompaction(ozoneManager.getConfiguration()); | ||||
| } | ||||
|
|
||||
| private List<String> validateTables(List<String> tables) { | ||||
|
|
@@ -109,6 +111,11 @@ public List<String> getCompactableTables() { | |||
| return compactableTables; | ||||
| } | ||||
|
|
||||
| @VisibleForTesting | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
| public ManagedCompactRangeOptions.BottommostLevelCompaction getBottommostLevelCompaction() { | ||||
| return bottommostLevelCompaction; | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Returns the number of manual compactions performed. | ||||
| * | ||||
|
|
@@ -142,13 +149,11 @@ private boolean shouldRun() { | |||
| * @return CompletableFuture that completes when compaction finishes | ||||
| */ | ||||
| public CompletableFuture<Void> compactTableAsync(String tableName) { | ||||
| return CompactDBUtil.compactTableAsync(omMetadataManager, tableName, | ||||
| ManagedCompactRangeOptions.BottommostLevelCompaction.kForce); | ||||
| return CompactDBUtil.compactTableAsync(omMetadataManager, tableName, bottommostLevelCompaction); | ||||
| } | ||||
|
|
||||
| protected void compactFully(String tableName) throws IOException { | ||||
| CompactDBUtil.compactTable(omMetadataManager, tableName, | ||||
| ManagedCompactRangeOptions.BottommostLevelCompaction.kForce); | ||||
| CompactDBUtil.compactTable(omMetadataManager, tableName, bottommostLevelCompaction); | ||||
| } | ||||
|
|
||||
| private class CompactTask implements BackgroundTask { | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.hadoop.ozone.om.service; | ||
|
|
||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION; | ||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION_DEFAULT; | ||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_DB_DIRS; | ||
| import static org.apache.hadoop.ozone.om.service.CompactDBUtil.getBottommostLevelCompaction; | ||
| import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; | ||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||
|
|
||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import org.apache.hadoop.hdds.conf.OzoneConfiguration; | ||
| import org.apache.hadoop.hdds.utils.db.managed.ManagedCompactRangeOptions; | ||
| import org.apache.hadoop.ozone.om.OMMetadataManager; | ||
| import org.apache.hadoop.ozone.om.OmMetadataManagerImpl; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.EnumSource; | ||
| import org.junit.jupiter.params.provider.ValueSource; | ||
|
|
||
| /** | ||
| * Tests for {@link CompactDBUtil}. | ||
| */ | ||
| class TestCompactDBUtil { | ||
|
|
||
| private OMMetadataManager omMetadataManager; | ||
|
|
||
| @BeforeEach | ||
| void setup(@TempDir File tempDir) throws Exception { | ||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||
| conf.set(OZONE_OM_DB_DIRS, tempDir.getAbsolutePath()); | ||
| omMetadataManager = new OmMetadataManagerImpl(conf, null); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @EnumSource(ManagedCompactRangeOptions.BottommostLevelCompaction.class) | ||
| void testCompactionAlgorithms(ManagedCompactRangeOptions.BottommostLevelCompaction bottommostLevelCompaction) { | ||
| assertDoesNotThrow(() -> | ||
| CompactDBUtil.compactTable(omMetadataManager, "keyTable", bottommostLevelCompaction)); | ||
| } | ||
|
|
||
| @Test | ||
| void testCompactInvalidColumnFamily() { | ||
| assertThrows(IOException.class, () -> | ||
| CompactDBUtil.compactTable(omMetadataManager, "nonExistentTable", | ||
| ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip)); | ||
| } | ||
|
|
||
| @Test | ||
| void testDefaultConfigValueMapsToKSkip() { | ||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip, | ||
| OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION_DEFAULT); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"", "kForceeee"}) | ||
| void testDefaultConfigKeyIsReadFromOzoneConfiguration(String compactionType) { | ||
| // unset or invalid values should use the default value | ||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||
| conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, compactionType); | ||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip, getBottommostLevelCompaction(conf)); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = {"kForce", " kForce", "kForce ", " kForce "}) | ||
| void testConfigKeyIsReadFromOzoneConfiguration(String compactionType) { | ||
| // have trailing spaces in the config values to ensure they are trimmed and handled correctly | ||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||
| conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, compactionType); | ||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kForce, getBottommostLevelCompaction(conf)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,8 +17,10 @@ | |||||
|
|
||||||
| package org.apache.hadoop.ozone.om.service; | ||||||
|
|
||||||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION; | ||||||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_COMPACTION_SERVICE_ENABLED; | ||||||
| import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_COMPACTION_SERVICE_RUN_INTERVAL; | ||||||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||||||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||||||
| import static org.junit.jupiter.api.Assertions.assertThrows; | ||||||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||||||
|
|
@@ -39,6 +41,7 @@ | |||||
| import org.apache.hadoop.hdds.server.ServerUtils; | ||||||
| import org.apache.hadoop.hdds.utils.db.DBConfigFromFile; | ||||||
| import org.apache.hadoop.hdds.utils.db.TypedTable; | ||||||
| import org.apache.hadoop.hdds.utils.db.managed.ManagedCompactRangeOptions; | ||||||
| import org.apache.hadoop.ozone.om.OMConfigKeys; | ||||||
| import org.apache.hadoop.ozone.om.OMMetadataManager; | ||||||
| import org.apache.hadoop.ozone.om.OzoneManager; | ||||||
|
|
@@ -74,6 +77,7 @@ void setup(@TempDir Path tempDir) { | |||||
| ozoneManager = mock(OzoneManager.class); | ||||||
| OMMetadataManager metadataManager = mock(OMMetadataManager.class); | ||||||
| when(ozoneManager.getMetadataManager()).thenReturn(metadataManager); | ||||||
| when(ozoneManager.getConfiguration()).thenReturn(conf); | ||||||
| TypedTable table = mock(TypedTable.class); | ||||||
|
|
||||||
| Set<String> tables = new HashSet<>(); | ||||||
|
|
@@ -159,6 +163,35 @@ public void testCompactFailure() { | |||||
| () -> getCompactionService(compactTables)); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testDefaultCompactionLevelIsKSkip() { | ||||||
| CompactionService compactionService = getCompactionService(Arrays.asList("keyTable", "fileTable")); | ||||||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip, | ||||||
| compactionService.getBottommostLevelCompaction()); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testConfiguredCompactionLevelKForce() { | ||||||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||||||
| conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 2); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| when(ozoneManager.getConfiguration()).thenReturn(conf); | ||||||
|
|
||||||
| CompactionService compactionService = getCompactionService(Arrays.asList("keyTable", "fileTable")); | ||||||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kForce, | ||||||
| compactionService.getBottommostLevelCompaction()); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testInvalidCompactionLevelFallsBackToDefault() { | ||||||
| OzoneConfiguration conf = new OzoneConfiguration(); | ||||||
| conf.setInt(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, 99); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any invalid string can be used here -
Suggested change
|
||||||
| when(ozoneManager.getConfiguration()).thenReturn(conf); | ||||||
|
|
||||||
| CompactionService compactionService = getCompactionService(Arrays.asList("keyTable", "fileTable")); | ||||||
| assertEquals(ManagedCompactRangeOptions.BottommostLevelCompaction.kSkip, | ||||||
| compactionService.getBottommostLevelCompaction()); | ||||||
| } | ||||||
|
|
||||||
| private CompactionService getCompactionService(List<String> compactTables) { | ||||||
| CompactionService compactionService = new CompactionService(ozoneManager, TimeUnit.MILLISECONDS, | ||||||
| TimeUnit.SECONDS.toMillis(SERVICE_INTERVAL), TimeUnit.SECONDS.toMillis(60), compactTables) { | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use
-as separator in last part of the property name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary?
bottommostlevelcompactionis the original name as perorg.rocksdb.CompactRangeOptions.BottommostLevelCompaction.Most importantly, searching for
bottommostlevelcompactionbrings up all instances of it in both code and configuration in my IDE. Adding a separator would break that.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BottommostLevelCompactionusesCamelCase. The same with lower case needs some separator, e.g.-(kebab-case) or_(snake_case) for readability. In config property names-is commonly used.