You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ma...@apache.org on 2021/06/25 23:44:00 UTC

[incubator-pinot] branch master updated: Make TableConfigTunerRegistry configurable to scan packages. (#7089)

This is an automated email from the ASF dual-hosted git repository.

mayanks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 064a492  Make TableConfigTunerRegistry configurable to scan packages. (#7089)
064a492 is described below

commit 064a49226454b714c60b47d808f4e8b9b36454da
Author: Mayank Shrivastava <ma...@apache.org>
AuthorDate: Fri Jun 25 16:43:40 2021 -0700

    Make TableConfigTunerRegistry configurable to scan packages. (#7089)
---
 .../pinot/controller/BaseControllerStarter.java    |  4 ++
 .../apache/pinot/controller/ControllerConf.java    | 48 +++++++++++++---------
 .../api/resources/PinotTableRestletResource.java   |  3 +-
 .../api/resources/TableConfigsRestletResource.java |  3 +-
 .../tuner/NoOpTableTableConfigTuner.java           |  4 +-
 .../controller}/tuner/RealTimeAutoIndexTuner.java  |  4 +-
 .../pinot/controller}/tuner/TableConfigTuner.java  |  2 +-
 .../tuner/TableConfigTunerRegistry.java            | 47 ++++++++++++++++-----
 .../controller/tuner/TableConfigTunerUtils.java    | 23 +++++------
 .../org/apache/pinot/controller}/tuner/Tuner.java  |  2 +-
 .../tuner/RealTimeAutoIndexTunerTest.java          |  7 +++-
 .../pinot/controller}/tuner/TunerRegistryTest.java |  7 +++-
 .../segment/local/utils/TableConfigUtils.java      | 15 -------
 13 files changed, 97 insertions(+), 72 deletions(-)

diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
index d3bb073..9ed2e7f 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/BaseControllerStarter.java
@@ -74,6 +74,7 @@ import org.apache.pinot.controller.helix.core.retention.RetentionManager;
 import org.apache.pinot.controller.helix.core.statemodel.LeadControllerResourceMasterSlaveStateModelFactory;
 import org.apache.pinot.controller.helix.core.util.HelixSetupUtils;
 import org.apache.pinot.controller.helix.starter.HelixConfig;
+import org.apache.pinot.controller.tuner.TableConfigTunerRegistry;
 import org.apache.pinot.controller.validation.BrokerResourceValidationManager;
 import org.apache.pinot.controller.validation.OfflineSegmentIntervalChecker;
 import org.apache.pinot.controller.validation.RealtimeSegmentValidationManager;
@@ -176,6 +177,9 @@ public abstract class BaseControllerStarter implements ServiceStartable {
       _executorService =
           Executors.newCachedThreadPool(new ThreadFactoryBuilder().setNameFormat("restapi-multiget-thread-%d").build());
     }
+
+    // Initialize the table config tuner registry.
+    TableConfigTunerRegistry.init(_config.getTableConfigTunerPackages());
   }
 
   private void inferHostnameIfNeeded(ControllerConf config) {
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
index 43f5754..80bd173 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java
@@ -39,9 +39,8 @@ import static org.apache.pinot.spi.utils.CommonConstants.Controller.DEFAULT_METR
 
 
 public class ControllerConf extends PinotConfiguration {
-  public static final List<String> SUPPORTED_PROTOCOLS = Arrays.asList(
-      CommonConstants.HTTP_PROTOCOL,
-      CommonConstants.HTTPS_PROTOCOL);
+  public static final List<String> SUPPORTED_PROTOCOLS =
+      Arrays.asList(CommonConstants.HTTP_PROTOCOL, CommonConstants.HTTPS_PROTOCOL);
 
   public static final String CONTROLLER_VIP_HOST = "controller.vip.host";
   public static final String CONTROLLER_VIP_PORT = "controller.vip.port";
@@ -66,6 +65,10 @@ public class ControllerConf extends PinotConfiguration {
   public static final String CONTROLLER_MODE = "controller.mode";
   public static final String LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY = "controller.resource.rebalance.strategy";
 
+  // Comma separated list of list of packages that contain TableConfigTuners to be added to the registry
+  public static final String TABLE_CONFIG_TUNER_PACKAGES = "controller.table.config.tuner.packages";
+  public static final String DEFAULT_TABLE_CONFIG_TUNER_PACKAGES = "org.apache.pinot";
+
   public enum ControllerMode {
     DUAL, PINOT_ONLY, HELIX_ONLY
   }
@@ -91,9 +94,10 @@ public class ControllerConf extends PinotConfiguration {
     public static final String STATUS_CHECKER_WAIT_FOR_PUSH_TIME_IN_SECONDS =
         "controller.statuschecker.waitForPushTimeInSeconds";
     public static final String TASK_MANAGER_FREQUENCY_IN_SECONDS = "controller.task.frequencyInSeconds";
-    public static final String MINION_INSTANCES_CLEANUP_TASK_FREQUENCY_IN_SECONDS = "controller.minion.instances.cleanup.task.frequencyInSeconds";
-    public static final String MINION_INSTANCES_CLEANUP_TASK_INITIAL_DELAY_SECONDS = "controller.minion.instances.cleanup.task.initialDelaySeconds";
-
+    public static final String MINION_INSTANCES_CLEANUP_TASK_FREQUENCY_IN_SECONDS =
+        "controller.minion.instances.cleanup.task.frequencyInSeconds";
+    public static final String MINION_INSTANCES_CLEANUP_TASK_INITIAL_DELAY_SECONDS =
+        "controller.minion.instances.cleanup.task.initialDelaySeconds";
 
     public static final String PINOT_TASK_MANAGER_SCHEDULER_ENABLED = "controller.task.scheduler.enabled";
     @Deprecated
@@ -191,7 +195,7 @@ public class ControllerConf extends PinotConfiguration {
   public ControllerConf() {
     super(new HashMap<>());
   }
-  
+
   public ControllerConf(Map<String, Object> baseProperties) {
     super(baseProperties);
   }
@@ -305,7 +309,7 @@ public class ControllerConf extends PinotConfiguration {
   public String getControllerPort() {
     return getProperty(CONTROLLER_PORT);
   }
-  
+
   public List<String> getControllerAccessProtocols() {
     return getProperty(CONTROLLER_ACCESS_PROTOCOLS,
         getControllerPort() == null ? Arrays.asList("http") : Arrays.asList());
@@ -394,7 +398,7 @@ public class ControllerConf extends PinotConfiguration {
 
             // No protocol defines a port as VIP. Fallback on legacy controller.port property.
             .orElseGet(this::getControllerPort));
-  }  
+  }
 
   public String getControllerVipProtocol() {
     return getSupportedProtocol(CONTROLLER_VIP_PROTOCOL);
@@ -439,9 +443,8 @@ public class ControllerConf extends PinotConfiguration {
    * @return
    */
   public int getRealtimeSegmentValidationFrequencyInSeconds() {
-    return Optional
-        .ofNullable(
-            getProperty(ControllerPeriodicTasksConf.REALTIME_SEGMENT_VALIDATION_FREQUENCY_IN_SECONDS, Integer.class))
+    return Optional.ofNullable(
+        getProperty(ControllerPeriodicTasksConf.REALTIME_SEGMENT_VALIDATION_FREQUENCY_IN_SECONDS, Integer.class))
 
         .orElseGet(() -> getProperty(ControllerPeriodicTasksConf.DEPRECATED_VALIDATION_MANAGER_FREQUENCY_IN_SECONDS,
             ControllerPeriodicTasksConf.DEFAULT_REALTIME_SEGMENT_VALIDATION_FREQUENCY_IN_SECONDS));
@@ -460,9 +463,8 @@ public class ControllerConf extends PinotConfiguration {
    * @return
    */
   public int getBrokerResourceValidationFrequencyInSeconds() {
-    return Optional
-        .ofNullable(
-            getProperty(ControllerPeriodicTasksConf.BROKER_RESOURCE_VALIDATION_FREQUENCY_IN_SECONDS, Integer.class))
+    return Optional.ofNullable(
+        getProperty(ControllerPeriodicTasksConf.BROKER_RESOURCE_VALIDATION_FREQUENCY_IN_SECONDS, Integer.class))
 
         .orElseGet(() -> getProperty(ControllerPeriodicTasksConf.DEPRECATED_VALIDATION_MANAGER_FREQUENCY_IN_SECONDS,
             ControllerPeriodicTasksConf.DEFAULT_BROKER_RESOURCE_VALIDATION_FREQUENCY_IN_SECONDS));
@@ -569,7 +571,8 @@ public class ControllerConf extends PinotConfiguration {
   }
 
   public void setMinionInstancesCleanupTaskFrequencyInSeconds(int frequencyInSeconds) {
-    setProperty(ControllerPeriodicTasksConf.MINION_INSTANCES_CLEANUP_TASK_FREQUENCY_IN_SECONDS, Integer.toString(frequencyInSeconds));
+    setProperty(ControllerPeriodicTasksConf.MINION_INSTANCES_CLEANUP_TASK_FREQUENCY_IN_SECONDS,
+        Integer.toString(frequencyInSeconds));
   }
 
   public long getMinionInstancesCleanupTaskInitialDelaySeconds() {
@@ -578,7 +581,8 @@ public class ControllerConf extends PinotConfiguration {
   }
 
   public void setMinionInstancesCleanupTaskInitialDelaySeconds(int initialDelaySeconds) {
-    setProperty(ControllerPeriodicTasksConf.MINION_INSTANCES_CLEANUP_TASK_INITIAL_DELAY_SECONDS, Integer.toString(initialDelaySeconds));
+    setProperty(ControllerPeriodicTasksConf.MINION_INSTANCES_CLEANUP_TASK_INITIAL_DELAY_SECONDS,
+        Integer.toString(initialDelaySeconds));
   }
 
   public int getDefaultTableMinReplicas() {
@@ -690,7 +694,8 @@ public class ControllerConf extends PinotConfiguration {
   }
 
   public String getLeadControllerResourceRebalanceStrategy() {
-    return getProperty(LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY, DEFAULT_LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY);
+    return getProperty(LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY,
+        DEFAULT_LEAD_CONTROLLER_RESOURCE_REBALANCE_STRATEGY);
   }
 
   public boolean getHLCTablesAllowed() {
@@ -709,6 +714,10 @@ public class ControllerConf extends PinotConfiguration {
     return getProperty(CONTROLLER_BROKER_PORT_OVERRIDE, -1);
   }
 
+  public List<String> getTableConfigTunerPackages() {
+    return Arrays.asList(getProperty(TABLE_CONFIG_TUNER_PACKAGES, DEFAULT_TABLE_CONFIG_TUNER_PACKAGES).split("\\s*,\\s*"));
+  }
+
   private long convertPeriodToSeconds(String timeStr) {
     long seconds;
     try {
@@ -722,8 +731,7 @@ public class ControllerConf extends PinotConfiguration {
 
   private String getSupportedProtocol(String property) {
     String value = getProperty(property, CommonConstants.HTTP_PROTOCOL);
-    Preconditions.checkArgument(SUPPORTED_PROTOCOLS.contains(value),
-        "Unsupported %s protocol '%s'", property, value);
+    Preconditions.checkArgument(SUPPORTED_PROTOCOLS.contains(value), "Unsupported %s protocol '%s'", property, value);
     return value;
   }
 }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 63341bc..1cf1ef0 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -65,6 +65,7 @@ import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.controller.helix.core.rebalance.RebalanceConfigConstants;
 import org.apache.pinot.controller.helix.core.rebalance.RebalanceResult;
 import org.apache.pinot.controller.recommender.RecommenderDriver;
+import org.apache.pinot.controller.tuner.TableConfigTunerUtils;
 import org.apache.pinot.controller.util.ConsumingSegmentInfoReader;
 import org.apache.pinot.segment.local.utils.TableConfigUtils;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -148,7 +149,7 @@ public class PinotTableRestletResource {
 
       Schema schema = _pinotHelixResourceManager.getSchemaForTableConfig(tableConfig);
 
-      TableConfigUtils.applyTunerConfig(tableConfig, schema);
+      TableConfigTunerUtils.applyTunerConfig(tableConfig, schema);
 
       // TableConfigUtils.validate(...) is used across table create/update.
       TableConfigUtils.validate(tableConfig, schema);
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
index c6ceb93..c1fa10b 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
@@ -52,6 +52,7 @@ import org.apache.pinot.controller.api.exception.ControllerApplicationException;
 import org.apache.pinot.controller.api.exception.InvalidTableConfigException;
 import org.apache.pinot.controller.api.exception.TableAlreadyExistsException;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.controller.tuner.TableConfigTunerUtils;
 import org.apache.pinot.segment.local.utils.SchemaUtils;
 import org.apache.pinot.segment.local.utils.TableConfigUtils;
 import org.apache.pinot.spi.config.TableConfigs;
@@ -351,7 +352,7 @@ public class TableConfigsRestletResource {
   }
 
   private void tuneConfig(TableConfig tableConfig, Schema schema) {
-    TableConfigUtils.applyTunerConfig(tableConfig, schema);
+    TableConfigTunerUtils.applyTunerConfig(tableConfig, schema);
     TableConfigUtils.ensureMinReplicas(tableConfig, _controllerConf.getDefaultTableMinReplicas());
     TableConfigUtils.ensureStorageQuotaConstraints(tableConfig, _controllerConf.getDimTableMaxSize());
   }
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/NoOpTableTableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
similarity index 88%
rename from pinot-common/src/main/java/org/apache/pinot/common/config/tuner/NoOpTableTableConfigTuner.java
rename to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
index 4575fb7..f99ecc6 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/NoOpTableTableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
@@ -16,12 +16,10 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.common.config.tuner;
+package org.apache.pinot.controller.tuner;
 
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TunerConfig;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
-import org.apache.pinot.spi.config.table.tuner.Tuner;
 import org.apache.pinot.spi.data.Schema;
 
 
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
similarity index 91%
rename from pinot-common/src/main/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTuner.java
rename to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
index 9cf787a..f0d7f3b 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
@@ -16,13 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.common.config.tuner;
+package org.apache.pinot.controller.tuner;
 
 import org.apache.pinot.spi.config.table.IndexingConfig;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TunerConfig;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
-import org.apache.pinot.spi.config.table.tuner.Tuner;
 import org.apache.pinot.spi.data.Schema;
 
 
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
similarity index 96%
copy from pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java
copy to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
index bed80f5..8105ca5 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.spi.config.table.tuner;
+package org.apache.pinot.controller.tuner;
 
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TunerConfig;
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/TableConfigTunerRegistry.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerRegistry.java
similarity index 65%
rename from pinot-common/src/main/java/org/apache/pinot/common/config/tuner/TableConfigTunerRegistry.java
rename to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerRegistry.java
index 12d62a3..ccba429 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/config/tuner/TableConfigTunerRegistry.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerRegistry.java
@@ -16,13 +16,15 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.common.config.tuner;
+package org.apache.pinot.controller.tuner;
 
-import java.util.HashMap;
+import com.google.common.base.Preconditions;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
-import org.apache.pinot.spi.config.table.tuner.Tuner;
+import java.util.concurrent.ConcurrentHashMap;
 import org.reflections.Reflections;
 import org.reflections.scanners.ResourcesScanner;
 import org.reflections.scanners.SubTypesScanner;
@@ -42,19 +44,39 @@ public class TableConfigTunerRegistry {
   }
 
   private static final Logger LOGGER = LoggerFactory.getLogger(TableConfigTunerRegistry.class);
-  private static final Map<String, TableConfigTuner> _configTunerMap = new HashMap<>();
+  private static final Map<String, TableConfigTuner> _configTunerMap = new ConcurrentHashMap<>();
+  private static boolean _init = false;
+
+  /**
+   * Init method that initializes the _configTunerMap with all available tuners.
+   * <ul>
+   *   <li>Scans all packages specified, for class paths that have 'tuner' in path.</li>
+   *   <li>Looks for {@link Tuner} annotation for classes and adds them to the map. </li>
+   *   <li>Also, asserts that init was not already called before.</li>
+   * </ul>
+   * @param packages Packages to scan.
+   */
+  public static void init(List<String> packages) {
+    if (_init) {
+      LOGGER.info("TableConfigTunerRegistry already initialized, skipping.");
+      return;
+    }
+    long startTime = System.currentTimeMillis();
+
+    List<URL> urls = new ArrayList<>();
+    for (String pack : packages) {
+      urls.addAll(ClasspathHelper.forPackage(pack));
+    }
 
-  static {
     Reflections reflections = new Reflections(
-        new ConfigurationBuilder().setUrls(ClasspathHelper.forPackage("org.apache.pinot"))
-            .filterInputsBy(new FilterBuilder.Include(".*\\.tuner\\..*"))
+        new ConfigurationBuilder().setUrls(urls).filterInputsBy(new FilterBuilder.Include(".*\\.tuner\\..*"))
             .setScanners(new ResourcesScanner(), new TypeAnnotationsScanner(), new SubTypesScanner()));
     Set<Class<?>> classes = reflections.getTypesAnnotatedWith(Tuner.class);
     classes.forEach(tunerClass -> {
       Tuner tunerAnnotation = tunerClass.getAnnotation(Tuner.class);
       if (tunerAnnotation.enabled()) {
         if (tunerAnnotation.name().isEmpty()) {
-          LOGGER.error("Cannot register an unnamed config tuner for annotation {} ", tunerAnnotation.toString());
+          LOGGER.error("Cannot register an unnamed config tuner for annotation {} ", tunerAnnotation);
         } else {
           String tunerName = tunerAnnotation.name();
           TableConfigTuner tuner;
@@ -67,11 +89,14 @@ public class TableConfigTunerRegistry {
         }
       }
     });
-    LOGGER.info("Initialized TableConfigTunerRegistry with {} tuners: {}", _configTunerMap.size(),
-        _configTunerMap.keySet());
+
+    _init = true;
+    LOGGER.info("Initialized TableConfigTunerRegistry with {} tuners: {} in {} ms", _configTunerMap.size(),
+        _configTunerMap.keySet(), (System.currentTimeMillis() - startTime));
   }
 
   public static TableConfigTuner getTuner(String name) {
+    Preconditions.checkState(_init, "TableConfigTunerRegistry not yet initialized.");
     return _configTunerMap.get(name);
   }
 }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
similarity index 64%
rename from pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java
rename to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
index bed80f5..ffc39d0 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/TableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
@@ -16,25 +16,24 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.spi.config.table.tuner;
+package org.apache.pinot.controller.tuner;
 
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TunerConfig;
 import org.apache.pinot.spi.data.Schema;
 
 
-/**
- * Interface for Table Config Tuner.
- */
-public interface TableConfigTuner {
-  /**
-   * Used to initialize underlying implementation with Schema
-   * and custom properties (eg: metrics end point)
-   */
-  void init(TunerConfig props, Schema schema);
+public class TableConfigTunerUtils {
 
   /**
-   * Takes the original TableConfig and returns a tuned one
+   * Apply TunerConfig to the tableConfig
    */
-  TableConfig apply(TableConfig initialConfig);
+  public static void applyTunerConfig(TableConfig tableConfig, Schema schema) {
+    TunerConfig tunerConfig = tableConfig.getTunerConfig();
+    if (tunerConfig != null && tunerConfig.getName() != null && !tunerConfig.getName().isEmpty()) {
+      TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(tunerConfig.getName());
+      tuner.init(tunerConfig, schema);
+      tuner.apply(tableConfig);
+    }
+  }
 }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/Tuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/Tuner.java
similarity index 96%
rename from pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/Tuner.java
rename to pinot-controller/src/main/java/org/apache/pinot/controller/tuner/Tuner.java
index 9b913a6..ef539ca 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/tuner/Tuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/Tuner.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.spi.config.table.tuner;
+package org.apache.pinot.controller.tuner;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTunerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
similarity index 92%
rename from pinot-common/src/test/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTunerTest.java
rename to pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
index fbe5740..771be72 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/config/tuner/RealTimeAutoIndexTunerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
@@ -16,8 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.common.config.tuner;
+package org.apache.pinot.controller.tuner;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -25,7 +26,6 @@ import org.apache.pinot.spi.config.table.IndexingConfig;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.config.table.TunerConfig;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
 import org.apache.pinot.spi.data.FieldSpec;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
@@ -33,6 +33,8 @@ import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import static org.apache.pinot.controller.ControllerConf.DEFAULT_TABLE_CONFIG_TUNER_PACKAGES;
+
 
 public class RealTimeAutoIndexTunerTest {
 
@@ -57,6 +59,7 @@ public class RealTimeAutoIndexTunerTest {
   public void testTuner() {
     TableConfig tableConfig =
         new TableConfigBuilder(TableType.OFFLINE).setTableName("test").setTunerConfig(_tunerConfig).build();
+    TableConfigTunerRegistry.init(Arrays.asList(DEFAULT_TABLE_CONFIG_TUNER_PACKAGES));
     TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(TUNER_NAME);
     tuner.init(_tunerConfig, schema);
     TableConfig result = tuner.apply(tableConfig);
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/config/tuner/TunerRegistryTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
similarity index 88%
rename from pinot-common/src/test/java/org/apache/pinot/common/config/tuner/TunerRegistryTest.java
rename to pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
index ae3c0ec..e740f1d 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/config/tuner/TunerRegistryTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
@@ -16,20 +16,22 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.pinot.common.config.tuner;
+package org.apache.pinot.controller.tuner;
 
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.config.table.TunerConfig;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
 import org.apache.pinot.spi.data.Schema;
 import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
+import static org.apache.pinot.controller.ControllerConf.DEFAULT_TABLE_CONFIG_TUNER_PACKAGES;
+
 
 public class TunerRegistryTest {
 
@@ -47,6 +49,7 @@ public class TunerRegistryTest {
     Schema schema = new Schema.SchemaBuilder().build();
     TableConfig tableConfig =
         new TableConfigBuilder(TableType.OFFLINE).setTableName("test").setTunerConfig(_tunerConfig).build();
+    TableConfigTunerRegistry.init(Arrays.asList(DEFAULT_TABLE_CONFIG_TUNER_PACKAGES));
     TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(TUNER_NAME);
     tuner.init(_tunerConfig, schema);
     TableConfig result = tuner.apply(tableConfig);
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
index ce88c15..a57ca84 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
@@ -30,7 +30,6 @@ import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
-import org.apache.pinot.common.config.tuner.TableConfigTunerRegistry;
 import org.apache.pinot.common.tier.TierFactory;
 import org.apache.pinot.common.utils.config.TagNameUtils;
 import org.apache.pinot.core.util.ReplicationUtils;
@@ -47,13 +46,11 @@ import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.config.table.TableTaskConfig;
 import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.config.table.TierConfig;
-import org.apache.pinot.spi.config.table.TunerConfig;
 import org.apache.pinot.spi.config.table.UpsertConfig;
 import org.apache.pinot.spi.config.table.ingestion.BatchIngestionConfig;
 import org.apache.pinot.spi.config.table.ingestion.FilterConfig;
 import org.apache.pinot.spi.config.table.ingestion.IngestionConfig;
 import org.apache.pinot.spi.config.table.ingestion.TransformConfig;
-import org.apache.pinot.spi.config.table.tuner.TableConfigTuner;
 import org.apache.pinot.spi.data.FieldSpec;
 import org.apache.pinot.spi.data.FieldSpec.DataType;
 import org.apache.pinot.spi.data.Schema;
@@ -658,18 +655,6 @@ public final class TableConfigUtils {
   }
 
   /**
-   * Apply TunerConfig to the tableConfig
-   */
-  public static void applyTunerConfig(TableConfig tableConfig, Schema schema) {
-    TunerConfig tunerConfig = tableConfig.getTunerConfig();
-    if (tunerConfig != null && tunerConfig.getName() != null && !tunerConfig.getName().isEmpty()) {
-      TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(tunerConfig.getName());
-      tuner.init(tunerConfig, schema);
-      tuner.apply(tableConfig);
-    }
-  }
-
-  /**
    * Ensure that the table config has the minimum number of replicas set as per cluster configs.
    * If is doesn't, set the required amount of replication in the table config
    */

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org