You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/06/27 21:40:29 UTC

[GitHub] [pinot] walterddr opened a new pull request, #8980: adding MultiStageEngineQuickStart

walterddr opened a new pull request, #8980:
URL: https://github.com/apache/pinot/pull/8980

   QuickStart to enable multi-stage engine. 


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] jackjlli commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910181632


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;
 
   public static final String KEY_OF_QUERY_RUNNER_HOSTNAME = "pinot.query.runner.hostname";
   public static final String DEFAULT_QUERY_RUNNER_HOSTNAME = "localhost";
   // query runner port is the mailbox port.
   public static final String KEY_OF_QUERY_RUNNER_PORT = "pinot.query.runner.port";
-  public static final int DEFAULT_QUERY_RUNNER_PORT = -1;
+  public static final int DEFAULT_QUERY_RUNNER_PORT = 0;

Review Comment:
   I see. That makes sense to set a default port to 0. Thanks for the explanation! Maybe we can add some description in Line 30 to note that `intermediate query runners can run on either broker or server`?



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r909182110


##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java:
##########
@@ -218,11 +218,12 @@ private Map<String, Object> getDefaultConfig(ServiceRole serviceRole)
             ControllerConf.ControllerMode.DUAL, true);
       case BROKER:
         return PinotConfigUtils
-            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT);
+            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT, -1);
       case SERVER:
         return PinotConfigUtils
             .generateServerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT,
-                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT, null, null);
+                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT, -1, -1, null,

Review Comment:
   sounds good. i will make default port 0 in QueryConfig



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r908882798


##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -183,8 +183,9 @@ public void init(PinotConfiguration serverConf)
     int dataTableVersion =
         _serverConf.getProperty(Server.CONFIG_OF_CURRENT_DATA_TABLE_VERSION, Server.DEFAULT_CURRENT_DATA_TABLE_VERSION);
     if (dataTableVersion > Server.DEFAULT_CURRENT_DATA_TABLE_VERSION) {
-      throw new UnsupportedOperationException("Setting experimental DataTable version newer than default via config "
-          + "is not allowed. Current default DataTable version: " + Server.DEFAULT_CURRENT_DATA_TABLE_VERSION);
+      LOGGER.warn("Setting experimental DataTable version newer than default via config could result in"
+          + " backward-compatibility issues. Current default DataTable version: "
+          + Server.DEFAULT_CURRENT_DATA_TABLE_VERSION);

Review Comment:
   We need to allow enabling newer default DATA_TABLE_VERSION because multi-stage engine is not backward compatible with V3 and older data table



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] codecov-commenter commented on pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8980:
URL: https://github.com/apache/pinot/pull/8980#issuecomment-1167974123

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8980](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4bf26f1) into [master](https://codecov.io/gh/apache/pinot/commit/22c81de5383b19671213e8377445e1b3c79342ec?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (22c81de) will **increase** coverage by `3.28%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #8980      +/-   ##
   ============================================
   + Coverage     62.97%   66.26%   +3.28%     
   + Complexity     4864     4694     -170     
   ============================================
     Files          1770     1361     -409     
     Lines         92727    68701   -24026     
     Branches      13946    10746    -3200     
   ============================================
   - Hits          58398    45522   -12876     
   + Misses        30089    19899   -10190     
   + Partials       4240     3280     -960     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests1 | `66.26% <ø> (-0.01%)` | :arrow_down: |
   | unittests2 | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ore/query/scheduler/resources/ResourceManager.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvcmVzb3VyY2VzL1Jlc291cmNlTWFuYWdlci5qYXZh) | `84.00% <0.00%> (-12.00%)` | :arrow_down: |
   | [...ders/forward/VarByteChunkMVForwardIndexReader.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvZm9yd2FyZC9WYXJCeXRlQ2h1bmtNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `93.05% <0.00%> (-2.78%)` | :arrow_down: |
   | [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `78.08% <0.00%> (-2.74%)` | :arrow_down: |
   | [.../aggregation/function/ModeAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9Nb2RlQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `87.56% <0.00%> (-0.55%)` | :arrow_down: |
   | [...ot/controller/api/resources/ZookeeperResource.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1pvb2tlZXBlclJlc291cmNlLmphdmE=) | | |
   | [...ntroller/helix/core/rebalance/RebalanceResult.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYmFsYW5jZS9SZWJhbGFuY2VSZXN1bHQuamF2YQ==) | | |
   | [.../recommender/exceptions/InvalidInputException.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9leGNlcHRpb25zL0ludmFsaWRJbnB1dEV4Y2VwdGlvbi5qYXZh) | | |
   | [...r/recommender/rules/impl/AggregateMetricsRule.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pbXBsL0FnZ3JlZ2F0ZU1ldHJpY3NSdWxlLmphdmE=) | | |
   | [...roller/helix/core/relocation/SegmentRelocator.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlbG9jYXRpb24vU2VnbWVudFJlbG9jYXRvci5qYXZh) | | |
   | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | | |
   | ... and [407 more](https://codecov.io/gh/apache/pinot/pull/8980/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [22c81de...4bf26f1](https://codecov.io/gh/apache/pinot/pull/8980?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr merged pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr merged PR #8980:
URL: https://github.com/apache/pinot/pull/8980


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] jackjlli commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r909053634


##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java:
##########
@@ -218,11 +218,12 @@ private Map<String, Object> getDefaultConfig(ServiceRole serviceRole)
             ControllerConf.ControllerMode.DUAL, true);
       case BROKER:
         return PinotConfigUtils
-            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT);
+            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT, -1);

Review Comment:
   Same here.



##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java:
##########
@@ -218,11 +218,12 @@ private Map<String, Object> getDefaultConfig(ServiceRole serviceRole)
             ControllerConf.ControllerMode.DUAL, true);
       case BROKER:
         return PinotConfigUtils
-            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT);
+            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT, -1);
       case SERVER:
         return PinotConfigUtils
             .generateServerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT,
-                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT, null, null);
+                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT, -1, -1, null,

Review Comment:
   Why not just use the default port value just as the other configs do, so that you don't have to validate whether the value is `<= 0`?



##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/QuickstartRunner.java:
##########
@@ -132,6 +136,7 @@ private void startBrokers()
     for (int i = 0; i < _numBrokers; i++) {
       StartBrokerCommand brokerStarter = new StartBrokerCommand();
       brokerStarter.setPort(DEFAULT_BROKER_PORT + i)
+          .setBrokerMultiStageRunnerPort(DEFAULT_BROKER_MULTISTAGE_RUNNER_PORT - i)

Review Comment:
   Can we use `+` instead of `-` since it's more intuitive? I know you did it because the `DEFAULT_SERVER_MULTISTAGE_RUNNER_PORT` is 8422. But can we update the value of `DEFAULT_SERVER_MULTISTAGE_RUNNER_PORT` to sth like 8442?



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910146031


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;
 
   public static final String KEY_OF_QUERY_RUNNER_HOSTNAME = "pinot.query.runner.hostname";
   public static final String DEFAULT_QUERY_RUNNER_HOSTNAME = "localhost";
   // query runner port is the mailbox port.
   public static final String KEY_OF_QUERY_RUNNER_PORT = "pinot.query.runner.port";
-  public static final int DEFAULT_QUERY_RUNNER_PORT = -1;
+  public static final int DEFAULT_QUERY_RUNNER_PORT = 0;

Review Comment:
   i am not sure this is the right solution b/c runner ports are not specific to the server or broker. 
   intermediate query runners can run on either broker or server, we just happened to only utilize the server component.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910146659


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;

Review Comment:
   same as above^



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910430024


##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.tools;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.tools.admin.PinotAdministrator;
+import org.apache.pinot.tools.admin.command.QuickstartRunner;
+
+
+public class MultistageEngineQuickStart extends QuickStartBase {
+
+  @Override
+  public List<String> types() {
+    return Collections.singletonList("MULTI_STAGE");
+  }
+
+  @Override
+  public Map<String, Object> getConfigOverrides() {
+    Map<String, Object> overrides = new HashMap<>(super.getConfigOverrides());
+    overrides.put("pinot.multistage.engine.enabled", "true");
+    overrides.put("pinot.server.instance.currentDataTableVersion", 4);
+    return overrides;
+  }
+
+  public void execute()
+      throws Exception {
+    File quickstartTmpDir = new File(_dataDir, String.valueOf(System.currentTimeMillis()));
+
+    // Baseball stat table

Review Comment:
   for now yes only one table. I will be modifying this to add more once the full operator support PRs are merged.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] ankitsultana commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910306135


##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.tools;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.tools.admin.PinotAdministrator;
+import org.apache.pinot.tools.admin.command.QuickstartRunner;
+
+
+public class MultistageEngineQuickStart extends QuickStartBase {
+
+  @Override
+  public List<String> types() {
+    return Collections.singletonList("MULTI_STAGE");
+  }
+
+  @Override
+  public Map<String, Object> getConfigOverrides() {
+    Map<String, Object> overrides = new HashMap<>(super.getConfigOverrides());
+    overrides.put("pinot.multistage.engine.enabled", "true");
+    overrides.put("pinot.server.instance.currentDataTableVersion", 4);
+    return overrides;
+  }
+
+  public void execute()
+      throws Exception {
+    File quickstartTmpDir = new File(_dataDir, String.valueOf(System.currentTimeMillis()));
+
+    // Baseball stat table
+    File baseBallStatsBaseDir = new File(quickstartTmpDir, "baseballStats");
+    File schemaFile = new File(baseBallStatsBaseDir, "baseballStats_schema.json");
+    File tableConfigFile = new File(baseBallStatsBaseDir, "baseballStats_offline_table_config.json");
+    File ingestionJobSpecFile = new File(baseBallStatsBaseDir, "ingestionJobSpec.yaml");
+    ClassLoader classLoader = Quickstart.class.getClassLoader();
+    URL resource = classLoader.getResource("examples/batch/baseballStats/baseballStats_schema.json");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, schemaFile);
+    resource = classLoader.getResource("examples/batch/baseballStats/baseballStats_offline_table_config.json");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, tableConfigFile);
+    resource = classLoader.getResource("examples/batch/baseballStats/ingestionJobSpec.yaml");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, ingestionJobSpecFile);
+    QuickstartTableRequest request = new QuickstartTableRequest(baseBallStatsBaseDir.getAbsolutePath());
+
+    File tempDir = new File(quickstartTmpDir, "tmp");
+    FileUtils.forceMkdir(tempDir);
+    QuickstartRunner runner =
+        new QuickstartRunner(Lists.newArrayList(request), 1, 1, 1, 1, tempDir, getConfigOverrides());
+
+    printStatus(Quickstart.Color.CYAN, "***** Starting Zookeeper, controller, broker and server *****");
+    runner.startAll();
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        printStatus(Quickstart.Color.GREEN, "***** Shutting down offline quick start *****");
+        runner.stop();
+        FileUtils.deleteDirectory(quickstartTmpDir);
+      } catch (Exception e) {
+        e.printStackTrace();
+      }
+    }));
+    printStatus(Quickstart.Color.CYAN, "***** Bootstrap baseballStats table *****");
+    runner.bootstrapTable();
+
+    waitForBootstrapToComplete(null);
+
+    printStatus(Quickstart.Color.YELLOW, "***** Offline quickstart setup complete *****");

Review Comment:
   nit: Multi Stage Engine quickstart setup complete



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] ankitsultana commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
ankitsultana commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910310947


##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.tools;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.tools.admin.PinotAdministrator;
+import org.apache.pinot.tools.admin.command.QuickstartRunner;
+
+
+public class MultistageEngineQuickStart extends QuickStartBase {
+
+  @Override
+  public List<String> types() {
+    return Collections.singletonList("MULTI_STAGE");
+  }
+
+  @Override
+  public Map<String, Object> getConfigOverrides() {
+    Map<String, Object> overrides = new HashMap<>(super.getConfigOverrides());
+    overrides.put("pinot.multistage.engine.enabled", "true");
+    overrides.put("pinot.server.instance.currentDataTableVersion", 4);
+    return overrides;
+  }
+
+  public void execute()
+      throws Exception {
+    File quickstartTmpDir = new File(_dataDir, String.valueOf(System.currentTimeMillis()));
+
+    // Baseball stat table

Review Comment:
   Will this Quickstart only have 1 table? Are you planning to add another table so we can demo the join functionality using the quickstart?
   
   Also, I will be adding some Uber themed use-case (with new datasets) in the table-group Quickstart. I think it would make sense to merge my Quickstart with this one.. I can take that up in a future PR. Lmk your thoughts.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910429598


##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.tools;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.tools.admin.PinotAdministrator;
+import org.apache.pinot.tools.admin.command.QuickstartRunner;
+
+
+public class MultistageEngineQuickStart extends QuickStartBase {
+
+  @Override
+  public List<String> types() {
+    return Collections.singletonList("MULTI_STAGE");
+  }
+
+  @Override
+  public Map<String, Object> getConfigOverrides() {
+    Map<String, Object> overrides = new HashMap<>(super.getConfigOverrides());
+    overrides.put("pinot.multistage.engine.enabled", "true");
+    overrides.put("pinot.server.instance.currentDataTableVersion", 4);
+    return overrides;
+  }
+
+  public void execute()
+      throws Exception {
+    File quickstartTmpDir = new File(_dataDir, String.valueOf(System.currentTimeMillis()));
+
+    // Baseball stat table
+    File baseBallStatsBaseDir = new File(quickstartTmpDir, "baseballStats");
+    File schemaFile = new File(baseBallStatsBaseDir, "baseballStats_schema.json");
+    File tableConfigFile = new File(baseBallStatsBaseDir, "baseballStats_offline_table_config.json");
+    File ingestionJobSpecFile = new File(baseBallStatsBaseDir, "ingestionJobSpec.yaml");
+    ClassLoader classLoader = Quickstart.class.getClassLoader();
+    URL resource = classLoader.getResource("examples/batch/baseballStats/baseballStats_schema.json");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, schemaFile);
+    resource = classLoader.getResource("examples/batch/baseballStats/baseballStats_offline_table_config.json");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, tableConfigFile);
+    resource = classLoader.getResource("examples/batch/baseballStats/ingestionJobSpec.yaml");
+    Preconditions.checkNotNull(resource);
+    FileUtils.copyURLToFile(resource, ingestionJobSpecFile);
+    QuickstartTableRequest request = new QuickstartTableRequest(baseBallStatsBaseDir.getAbsolutePath());
+
+    File tempDir = new File(quickstartTmpDir, "tmp");
+    FileUtils.forceMkdir(tempDir);
+    QuickstartRunner runner =
+        new QuickstartRunner(Lists.newArrayList(request), 1, 1, 1, 1, tempDir, getConfigOverrides());
+
+    printStatus(Quickstart.Color.CYAN, "***** Starting Zookeeper, controller, broker and server *****");
+    runner.startAll();
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        printStatus(Quickstart.Color.GREEN, "***** Shutting down offline quick start *****");
+        runner.stop();
+        FileUtils.deleteDirectory(quickstartTmpDir);
+      } catch (Exception e) {
+        e.printStackTrace();
+      }
+    }));
+    printStatus(Quickstart.Color.CYAN, "***** Bootstrap baseballStats table *****");
+    runner.bootstrapTable();
+
+    waitForBootstrapToComplete(null);
+
+    printStatus(Quickstart.Color.YELLOW, "***** Offline quickstart setup complete *****");

Review Comment:
   ```suggestion
       printStatus(Quickstart.Color.YELLOW, "***** Multi-stage engine quickstart setup complete *****");
   ```



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r909181682


##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/QuickstartRunner.java:
##########
@@ -132,6 +136,7 @@ private void startBrokers()
     for (int i = 0; i < _numBrokers; i++) {
       StartBrokerCommand brokerStarter = new StartBrokerCommand();
       brokerStarter.setPort(DEFAULT_BROKER_PORT + i)
+          .setBrokerMultiStageRunnerPort(DEFAULT_BROKER_MULTISTAGE_RUNNER_PORT - i)

Review Comment:
   sounds good



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] jackjlli commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r909264714


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;
 
   public static final String KEY_OF_QUERY_RUNNER_HOSTNAME = "pinot.query.runner.hostname";
   public static final String DEFAULT_QUERY_RUNNER_HOSTNAME = "localhost";
   // query runner port is the mailbox port.
   public static final String KEY_OF_QUERY_RUNNER_PORT = "pinot.query.runner.port";
-  public static final int DEFAULT_QUERY_RUNNER_PORT = -1;
+  public static final int DEFAULT_QUERY_RUNNER_PORT = 0;

Review Comment:
   Can we specify two default runner ports, one for broker and the other one for server? By doing so, you can just reuse the default port numbers (i.e. `8421` and `8442`) for both broker and server.



##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;

Review Comment:
   And here you can just use `8842`.



##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/StartServiceManagerCommand.java:
##########
@@ -218,11 +219,13 @@ private Map<String, Object> getDefaultConfig(ServiceRole serviceRole)
             ControllerConf.ControllerMode.DUAL, true);
       case BROKER:
         return PinotConfigUtils
-            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT);
+            .generateBrokerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_BROKER_QUERY_PORT,
+                QueryConfig.DEFAULT_QUERY_RUNNER_PORT);
       case SERVER:
         return PinotConfigUtils
             .generateServerConf(_clusterName, _zkAddress, null, CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT,
-                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT, null, null);
+                CommonConstants.Server.DEFAULT_ADMIN_API_PORT, CommonConstants.Server.DEFAULT_GRPC_PORT,
+                QueryConfig.DEFAULT_QUERY_SERVER_PORT, QueryConfig.DEFAULT_QUERY_RUNNER_PORT, null, null);

Review Comment:
   Same here and Line 223. Specifying two different default ports for both broker and server would be much clearer.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910430024


##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.tools;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import java.io.File;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.tools.admin.PinotAdministrator;
+import org.apache.pinot.tools.admin.command.QuickstartRunner;
+
+
+public class MultistageEngineQuickStart extends QuickStartBase {
+
+  @Override
+  public List<String> types() {
+    return Collections.singletonList("MULTI_STAGE");
+  }
+
+  @Override
+  public Map<String, Object> getConfigOverrides() {
+    Map<String, Object> overrides = new HashMap<>(super.getConfigOverrides());
+    overrides.put("pinot.multistage.engine.enabled", "true");
+    overrides.put("pinot.server.instance.currentDataTableVersion", 4);
+    return overrides;
+  }
+
+  public void execute()
+      throws Exception {
+    File quickstartTmpDir = new File(_dataDir, String.valueOf(System.currentTimeMillis()));
+
+    // Baseball stat table

Review Comment:
   for now yes only one table, self-join can be used to demo join functionality already.
   
   - I will be modifying this to add more once the full operator support PRs are merged. 
   - and yes table group quickstart can also be part of this by adding tables with table group. 
   



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [pinot] walterddr commented on a diff in pull request #8980: adding MultiStageEngineQuickStart

Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #8980:
URL: https://github.com/apache/pinot/pull/8980#discussion_r910429322


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/QueryConfig.java:
##########
@@ -23,13 +23,13 @@
  */
 public class QueryConfig {
   public static final String KEY_OF_QUERY_SERVER_PORT = "pinot.query.server.port";
-  public static final int DEFAULT_QUERY_SERVER_PORT = -1;
+  public static final int DEFAULT_QUERY_SERVER_PORT = 0;
 
   public static final String KEY_OF_QUERY_RUNNER_HOSTNAME = "pinot.query.runner.hostname";
   public static final String DEFAULT_QUERY_RUNNER_HOSTNAME = "localhost";
   // query runner port is the mailbox port.
   public static final String KEY_OF_QUERY_RUNNER_PORT = "pinot.query.runner.port";
-  public static final int DEFAULT_QUERY_RUNNER_PORT = -1;
+  public static final int DEFAULT_QUERY_RUNNER_PORT = 0;

Review Comment:
   for now I think we can just leave it since this is a config file. the comment is already there in QueryRunner



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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