kfaraz commented on code in PR #16247:
URL: https://github.com/apache/druid/pull/16247#discussion_r1556793273
##########
server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinatorConfig.java:
##########
@@ -47,9 +45,15 @@ public abstract class DruidCoordinatorConfig
@Default("false")
public abstract boolean isKillUnusedSegmentsEnabled();
+ /**
+ * @return the coordinator's kill period duration. If the config isn't
explicitly set,
+ * it defaults to the coordinator's indexing period.
Review Comment:
"coordinator's kill period" is very misleading. In fact, we should even
rename the method but maybe not needed in this PR. There is another open PR
which rewires all of these configs, where this will be addressed: #15705
```suggestion
* @return The period at which the coordinator cleans up unused segments.
If the config isn't explicitly set,
* it defaults to the coordinator's indexing period.
```
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java:
##########
@@ -245,17 +245,6 @@ private void killUnusedSegments(
stats.add(Stats.Kill.SUBMITTED_TASKS, submittedTasks);
}
- /**
- * <p>
- * Calculates the interval for which segments are to be killed in a
datasource.
Review Comment:
Why is this javadoc not valid anymore?
##########
server/src/main/java/org/apache/druid/server/coordinator/duty/KillUnusedSegments.java:
##########
@@ -145,7 +145,7 @@ public KillUnusedSegments(
@Override
public DruidCoordinatorRuntimeParams run(final DruidCoordinatorRuntimeParams
params)
{
- if (!canDutyRun()) {
Review Comment:
It was nice to have this method.
--
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]
