gargvishesh commented on code in PR #16810:
URL: https://github.com/apache/druid/pull/16810#discussion_r1695666189
##########
server/src/main/java/org/apache/druid/server/coordinator/DruidCompactionConfig.java:
##########
@@ -126,7 +128,36 @@ public boolean isUseAutoScaleSlots()
@JsonProperty
public CompactionEngine getEngine()
{
- return compactionEngine;
+ return engine;
+ }
+
+
+ // Null-safe getters not used for serialization
+ public ClusterCompactionConfig clusterConfig()
+ {
+ return new ClusterCompactionConfig(
+ compactionTaskSlotRatio,
+ maxCompactionTaskSlots,
+ useAutoScaleSlots,
+ engine
+ );
+ }
+
+ public Map<String, DataSourceCompactionConfig> dataSourceToCompactionConfig()
Review Comment:
The name seems to suggest conversion when it actually returns a map.
```suggestion
public Map<String, DataSourceCompactionConfig>
dataSourceToCompactionConfigMap()
```
##########
server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigAuditEntry.java:
##########
@@ -50,7 +51,7 @@ public DataSourceCompactionConfigAuditEntry(
}
@JsonProperty
- public GlobalCompactionConfig getGlobalConfig()
+ public ClusterCompactionConfig getGlobalConfig()
Review Comment:
```suggestion
public ClusterCompactionConfig getClusterCompactionConfig()
```
##########
server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigAuditEntry.java:
##########
@@ -20,24 +20,25 @@
package org.apache.druid.server.coordinator;
import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.druid.audit.AuditInfo;
import org.joda.time.DateTime;
+import java.util.Objects;
+
/**
* A DTO containing audit information for compaction config for a datasource.
*/
public class DataSourceCompactionConfigAuditEntry
{
- private final GlobalCompactionConfig globalConfig;
+ private final ClusterCompactionConfig globalConfig;
private final DataSourceCompactionConfig compactionConfig;
private final AuditInfo auditInfo;
private final DateTime auditTime;
@JsonCreator
public DataSourceCompactionConfigAuditEntry(
- @JsonProperty("globalConfig") GlobalCompactionConfig globalConfig,
+ @JsonProperty("globalConfig") ClusterCompactionConfig globalConfig,
Review Comment:
```suggestion
@JsonProperty("globalConfig") ClusterCompactionConfig
clusterCompactionConfig,
```
##########
server/src/main/java/org/apache/druid/server/coordinator/DataSourceCompactionConfigAuditEntry.java:
##########
@@ -20,24 +20,25 @@
package org.apache.druid.server.coordinator;
import com.fasterxml.jackson.annotation.JsonCreator;
-import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.druid.audit.AuditInfo;
import org.joda.time.DateTime;
+import java.util.Objects;
+
/**
* A DTO containing audit information for compaction config for a datasource.
*/
public class DataSourceCompactionConfigAuditEntry
{
- private final GlobalCompactionConfig globalConfig;
+ private final ClusterCompactionConfig globalConfig;
Review Comment:
Consistent naming may be better. Can use older annotation for getter
property name.
```suggestion
private final ClusterCompactionConfig clusterCompactionConfig;
```
##########
server/src/main/java/org/apache/druid/server/coordinator/DruidCompactionConfig.java:
##########
@@ -126,7 +128,36 @@ public boolean isUseAutoScaleSlots()
@JsonProperty
public CompactionEngine getEngine()
{
- return compactionEngine;
+ return engine;
+ }
+
+
+ // Null-safe getters not used for serialization
+ public ClusterCompactionConfig clusterConfig()
+ {
+ return new ClusterCompactionConfig(
+ compactionTaskSlotRatio,
+ maxCompactionTaskSlots,
+ useAutoScaleSlots,
+ engine
+ );
+ }
+
+ public Map<String, DataSourceCompactionConfig> dataSourceToCompactionConfig()
+ {
+ return getCompactionConfigs().stream().collect(
+ Collectors.toMap(DataSourceCompactionConfig::getDataSource,
Function.identity())
+ );
+ }
+
+ public Optional<DataSourceCompactionConfig> findConfigForDatasource(String
dataSource)
Review Comment:
Thanks for this handy function!
##########
integration-tests/src/test/java/org/apache/druid/tests/coordinator/duty/ITAutoCompactionUpgradeTest.java:
##########
@@ -58,67 +56,56 @@ public void
testUpgradeAutoCompactionConfigurationWhenConfigurationFromOlderVers
{
// Verify that compaction config already exist. This config was inserted
manually into the database using SQL script.
// This auto compaction configuration payload is from Druid 0.21.0
- CoordinatorCompactionConfig coordinatorCompactionConfig =
compactionResource.getCoordinatorCompactionConfigs();
- DataSourceCompactionConfig foundDataSourceCompactionConfig = null;
- for (DataSourceCompactionConfig dataSourceCompactionConfig :
coordinatorCompactionConfig.getCompactionConfigs()) {
- if
(dataSourceCompactionConfig.getDataSource().equals(UPGRADE_DATASOURCE_NAME)) {
- foundDataSourceCompactionConfig = dataSourceCompactionConfig;
- }
- }
+ DruidCompactionConfig coordinatorCompactionConfig =
compactionResource.getCompactionConfig();
+ DataSourceCompactionConfig foundDataSourceCompactionConfig
+ =
coordinatorCompactionConfig.findConfigForDatasource(UPGRADE_DATASOURCE_NAME).orNull();
Assert.assertNotNull(foundDataSourceCompactionConfig);
// Now submit a new auto compaction configuration
PartitionsSpec newPartitionsSpec = new DynamicPartitionsSpec(4000, null);
Period newSkipOffset = Period.seconds(0);
- DataSourceCompactionConfig compactionConfig = new
DataSourceCompactionConfig(
- UPGRADE_DATASOURCE_NAME,
- null,
- null,
- null,
- newSkipOffset,
- new UserCompactionTaskQueryTuningConfig(
- null,
- null,
- null,
- null,
- new MaxSizeSplitHintSpec(null, 1),
- newPartitionsSpec,
- null,
- null,
- null,
- null,
- null,
- 1,
- null,
- null,
- null,
- null,
- null,
- 1,
- null
- ),
- new UserCompactionTaskGranularityConfig(Granularities.YEAR, null,
null),
- null,
- null,
- null,
- new UserCompactionTaskIOConfig(true),
- null,
- null
- );
+ DataSourceCompactionConfig compactionConfig = DataSourceCompactionConfig
+ .builder()
+ .forDataSource(UPGRADE_DATASOURCE_NAME)
+ .withSkipOffsetFromLatest(newSkipOffset)
+ .withTuningConfig(
+ new UserCompactionTaskQueryTuningConfig(
Review Comment:
A builder can be introduced for this one as well in future.
--
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]
