sarvekshayr commented on code in PR #10419:
URL: https://github.com/apache/ozone/pull/10419#discussion_r3578223527
##########
hadoop-hdds/common/src/main/resources/ozone-default.xml:
##########
@@ -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>
+ <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>
Review Comment:
`getEnum` expects enum names with this change. Please update the value as
well as the description.
```suggestion
<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>
```
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java:
##########
@@ -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).
Review Comment:
```suggestion
* Bottommost level compaction type for background compaction service.
* Invalid values will default to kSkip.
* Valid values: kSkip, kIfHaveCompactionFilter, kForce, kForceOptimized.
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestCompactionService.java:
##########
@@ -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);
+ 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);
Review Comment:
Any invalid string can be used here -
```suggestion
conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION, "abc");
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/service/TestCompactionService.java:
##########
@@ -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);
Review Comment:
```suggestion
conf.set(OZONE_OM_COMPACTION_SERVICE_BOTTOMMOSTLEVELCOMPACTION,
"kForce");
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/CompactionService.java:
##########
@@ -109,6 +111,11 @@ public List<String> getCompactableTables() {
return compactableTables;
}
+ @VisibleForTesting
Review Comment:
`@VisibleForTesting` annotation is no longer used, see
[HDDS-12725](https://issues.apache.org/jira/browse/HDDS-12725).
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
