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

[pinot] branch master updated: add extra properties to the tuner, since tuner right now doesn't take TunerConfig properties (#7621)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new c15d2f0  add extra properties to the tuner, since tuner right now doesn't take TunerConfig properties (#7621)
c15d2f0 is described below

commit c15d2f0095c3bc7103d0e20d6d09c43df4f8e55d
Author: Rong Rong <wa...@gmail.com>
AuthorDate: Mon Oct 25 16:22:26 2021 -0700

    add extra properties to the tuner, since tuner right now doesn't take TunerConfig properties (#7621)
    
    We made tuner stateless a while ago so now it is requires some extra properties for run. for example some of the unused properties set by TunerConfig.
    
    This adds additional map field at the end of the Tuner apply API.
---
 .../controller/api/resources/PinotTableRestletResource.java      | 3 ++-
 .../controller/api/resources/TableConfigsRestletResource.java    | 2 +-
 .../apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java | 3 ++-
 .../apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java    | 3 ++-
 .../java/org/apache/pinot/controller/tuner/TableConfigTuner.java | 7 ++++---
 .../org/apache/pinot/controller/tuner/TableConfigTunerUtils.java | 9 +++++----
 .../pinot/controller/tuner/RealTimeAutoIndexTunerTest.java       | 3 ++-
 .../org/apache/pinot/controller/tuner/TunerRegistryTest.java     | 3 ++-
 8 files changed, 20 insertions(+), 13 deletions(-)

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 a15a824..7c22851 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
@@ -30,6 +30,7 @@ import it.unimi.dsi.fastutil.Swapper;
 import it.unimi.dsi.fastutil.ints.IntComparator;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.LinkedList;
 import java.util.List;
@@ -167,7 +168,7 @@ public class PinotTableRestletResource {
 
       Schema schema = _pinotHelixResourceManager.getSchemaForTableConfig(tableConfig);
 
-      TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager, tableConfig, schema);
+      TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager, tableConfig, schema, Collections.emptyMap());
 
       // 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 529de19..0713a94 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
@@ -361,7 +361,7 @@ public class TableConfigsRestletResource {
   }
 
   private void tuneConfig(TableConfig tableConfig, Schema schema) {
-    TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager, tableConfig, schema);
+    TableConfigTunerUtils.applyTunerConfigs(_pinotHelixResourceManager, tableConfig, schema, Collections.emptyMap());
     TableConfigUtils.ensureMinReplicas(tableConfig, _controllerConf.getDefaultTableMinReplicas());
     TableConfigUtils.ensureStorageQuotaConstraints(tableConfig, _controllerConf.getDimTableMaxSize());
   }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
index 47d1a97..f8e18a7 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/NoOpTableTableConfigTuner.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.controller.tuner;
 
+import java.util.Map;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.table.TableConfig;
 import org.apache.pinot.spi.data.Schema;
@@ -28,7 +29,7 @@ public class NoOpTableTableConfigTuner implements TableConfigTuner {
 
   @Override
   public TableConfig apply(PinotHelixResourceManager pinotHelixResourceManager,
-      TableConfig initialConfig, Schema schema) {
+      TableConfig initialConfig, Schema schema, Map<String, String> extraProperties) {
     return initialConfig;
   }
 }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
index add2f86..a1d25eb 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTuner.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.controller.tuner;
 
+import java.util.Map;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.table.IndexingConfig;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -35,7 +36,7 @@ public class RealTimeAutoIndexTuner implements TableConfigTuner {
 
   @Override
   public TableConfig apply(PinotHelixResourceManager pinotHelixResourceManager,
-      TableConfig tableConfig, Schema schema) {
+      TableConfig tableConfig, Schema schema, Map<String, String> extraProperties) {
     IndexingConfig initialIndexingConfig = tableConfig.getIndexingConfig();
     initialIndexingConfig.setInvertedIndexColumns(schema.getDimensionNames());
     initialIndexingConfig.setNoDictionaryColumns(schema.getMetricNames());
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
index c8084e5..3f84ce1 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTuner.java
@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.controller.tuner;
 
+import java.util.Map;
 import javax.annotation.Nullable;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -32,13 +33,13 @@ import org.apache.yetus.audience.InterfaceStability;
 public interface TableConfigTuner {
 
   /**
-   * Used to initialize underlying implementation with Schema
-   * and custom properties (eg: metrics end point)
+   * Apply tuner to a {@link TableConfig}.
    *
    * @param pinotHelixResourceManager Pinot Helix Resource Manager to access Helix resources
    * @param tableConfig tableConfig that needs to be tuned.
    * @param schema Table schema
+   * @param extraProperties extraProperties for the tuner implementation.
    */
   TableConfig apply(@Nullable PinotHelixResourceManager pinotHelixResourceManager,
-      TableConfig tableConfig, Schema schema);
+      TableConfig tableConfig, Schema schema, Map<String, String> extraProperties);
 }
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
index 4ef0f86..33c7b40 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/tuner/TableConfigTunerUtils.java
@@ -19,6 +19,7 @@
 package org.apache.pinot.controller.tuner;
 
 import java.util.List;
+import java.util.Map;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -38,11 +39,11 @@ public class TableConfigTunerUtils {
    * Apply the entire tunerConfig list inside the tableConfig.
    */
   public static void applyTunerConfigs(PinotHelixResourceManager pinotHelixResourceManager, TableConfig tableConfig,
-      Schema schema) {
+      Schema schema, Map<String, String> extraProperties) {
     List<TunerConfig> tunerConfigs = tableConfig.getTunerConfigsList();
     if (CollectionUtils.isNotEmpty(tunerConfigs)) {
       for (TunerConfig tunerConfig : tunerConfigs) {
-        applyTunerConfig(pinotHelixResourceManager, tunerConfig, tableConfig, schema);
+        applyTunerConfig(pinotHelixResourceManager, tunerConfig, tableConfig, schema, extraProperties);
       }
     }
   }
@@ -52,11 +53,11 @@ public class TableConfigTunerUtils {
    */
   public static void applyTunerConfig(
       PinotHelixResourceManager pinotHelixResourceManager, TunerConfig tunerConfig, TableConfig tableConfig,
-      Schema schema) {
+      Schema schema, Map<String, String> extraProperties) {
     if (tunerConfig != null && tunerConfig.getName() != null && !tunerConfig.getName().isEmpty()) {
       try {
         TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(tunerConfig.getName());
-        tuner.apply(pinotHelixResourceManager, tableConfig, schema);
+        tuner.apply(pinotHelixResourceManager, tableConfig, schema, extraProperties);
       } catch (Exception e) {
         LOGGER.error(String.format("Exception occur when applying tuner: %s", tunerConfig.getName()), e);
       }
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
index a2f7660..f172fe3 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/RealTimeAutoIndexTunerTest.java
@@ -19,6 +19,7 @@
 package org.apache.pinot.controller.tuner;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -62,7 +63,7 @@ public class RealTimeAutoIndexTunerTest {
         .setTableName("test").setTunerConfigList(Arrays.asList(_tunerConfig)).build();
     TableConfigTunerRegistry.init(Arrays.asList(DEFAULT_TABLE_CONFIG_TUNER_PACKAGES));
     TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(TUNER_NAME);
-    TableConfig result = tuner.apply(null, tableConfig, _schema);
+    TableConfig result = tuner.apply(null, tableConfig, _schema, Collections.emptyMap());
 
     IndexingConfig newConfig = result.getIndexingConfig();
     List<String> invertedIndexColumns = newConfig.getInvertedIndexColumns();
diff --git a/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
index 004dce7..5da7806 100644
--- a/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
+++ b/pinot-controller/src/test/java/org/apache/pinot/controller/tuner/TunerRegistryTest.java
@@ -19,6 +19,7 @@
 package org.apache.pinot.controller.tuner;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import org.apache.pinot.spi.config.table.TableConfig;
@@ -51,7 +52,7 @@ public class TunerRegistryTest {
         .setTableName("test").setTunerConfigList(Arrays.asList(_tunerConfig)).build();
     TableConfigTunerRegistry.init(Arrays.asList(DEFAULT_TABLE_CONFIG_TUNER_PACKAGES));
     TableConfigTuner tuner = TableConfigTunerRegistry.getTuner(TUNER_NAME);
-    TableConfig result = tuner.apply(null, tableConfig, schema);
+    TableConfig result = tuner.apply(null, tableConfig, schema, Collections.emptyMap());
     Assert.assertEquals(result, tableConfig);
   }
 }

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