Enable druid.coordinator.kill.pendingSegments.on by default (#9385)

* Enable druid.coordinator.kill.pendingSegments.on by default

* checkstyle

core/src/main/java/org/apache/druid/guice/ConditionalMultibind.java

@@ -25,6 +25,7 @@ import com.google.inject.TypeLiteral;
 import com.google.inject.multibindings.Multibinder;
 import org.apache.druid.guice.annotations.PublicApi;
 
+import javax.annotation.Nullable;
 import java.lang.annotation.Annotation;
 import java.util.Properties;
 
@@ -184,7 +185,17 @@ public class ConditionalMultibind<T>
       Class<? extends T> target
   )
   {
-    if (matchCondition(property, condition)) {
+    return addConditionBinding(property, null, condition, target);
+  }
+
+  public ConditionalMultibind<T> addConditionBinding(
+      String property,
+      String defaultValue,
+      Predicate<String> condition,
+      Class<? extends T> target
+  )
+  {
+    if (matchCondition(property, defaultValue, condition)) {
       multibinder.addBinding().to(target);
     }
     return this;
@@ -235,7 +246,12 @@ public class ConditionalMultibind<T>
 
   public boolean matchCondition(String property, Predicate<String> condition)
   {
-    final String value = properties.getProperty(property);
+    return matchCondition(property, null, condition);
+  }
+
+  public boolean matchCondition(String property, @Nullable String defaultValue, Predicate<String> condition)
+  {
+    final String value = properties.getProperty(property, defaultValue);
     if (value == null) {
       return false;
     }

docs/configuration/index.md

@@ -678,7 +678,7 @@ These Coordinator static configurations can be defined in the `coordinator/runti
 |`druid.coordinator.period.indexingPeriod`|How often to send compact/merge/conversion tasks to the indexing service. It's recommended to be longer than `druid.manager.segments.pollDuration`|PT1800S (30 mins)|
 |`druid.coordinator.startDelay`|The operation of the Coordinator works on the assumption that it has an up-to-date view of the state of the world when it runs, the current ZK interaction code, however, is written in a way that doesn’t allow the Coordinator to know for a fact that it’s done loading the current state of the world. This delay is a hack to give it enough time to believe that it has all the data.|PT300S|
 |`druid.coordinator.load.timeout`|The timeout duration for when the Coordinator assigns a segment to a Historical process.|PT15M|
-|`druid.coordinator.kill.pendingSegments.on`|Boolean flag for whether or not the Coordinator clean up old entries in the `pendingSegments` table of metadata store. If set to true, Coordinator will check the created time of most recently complete task. If it doesn't exist, it finds the created time of the earliest running/pending/waiting tasks. Once the created time is found, then for all dataSources not in the `killPendingSegmentsSkipList` (see [Dynamic configuration](#dynamic-configuration)), Coordinator will ask the Overlord to clean up the entries 1 day or more older than the found created time in the `pendingSegments` table. This will be done periodically based on `druid.coordinator.period` specified.|false|
+|`druid.coordinator.kill.pendingSegments.on`|Boolean flag for whether or not the Coordinator clean up old entries in the `pendingSegments` table of metadata store. If set to true, Coordinator will check the created time of most recently complete task. If it doesn't exist, it finds the created time of the earliest running/pending/waiting tasks. Once the created time is found, then for all dataSources not in the `killPendingSegmentsSkipList` (see [Dynamic configuration](#dynamic-configuration)), Coordinator will ask the Overlord to clean up the entries 1 day or more older than the found created time in the `pendingSegments` table. This will be done periodically based on `druid.coordinator.period` specified.|true|
 |`druid.coordinator.kill.on`|Boolean flag for whether or not the Coordinator should submit kill task for unused segments, that is, hard delete them from metadata store and deep storage. If set to true, then for all whitelisted dataSources (or optionally all), Coordinator will submit tasks periodically based on `period` specified. These kill tasks will delete all segments except for the last `durationToRetain` period. Whitelist or All can be set via dynamic configuration `killAllDataSources` and `killDataSourceWhitelist` described later.|false|
 |`druid.coordinator.kill.period`|How often to send kill tasks to the indexing service. Value must be greater than `druid.coordinator.period.indexingPeriod`. Only applies if kill is turned on.|P1D (1 Day)|
 |`druid.coordinator.kill.durationToRetain`| Do not kill segments in last `durationToRetain`, must be greater or equal to 0. Only applies and MUST be specified if kill is turned on. Note that default value is invalid.|PT-1S (-1 seconds)|

services/src/main/java/org/apache/druid/cli/CliCoordinator.java

@@ -232,8 +232,10 @@ public class CliCoordinator extends ServerRunnable
                 "druid.coordinator.kill.on",
                 Predicates.equalTo("true"),
                 KillUnusedSegments.class
-            ).addConditionBinding(
+            );
+            conditionalMultibind.addConditionBinding(
                 "druid.coordinator.kill.pendingSegments.on",
+                "true",
                 Predicates.equalTo("true"),
                 KillStalePendingSegments.class
             );