You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2023/04/04 21:37:31 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #10543: Turn on v2 engine by default
xiangfu0 opened a new pull request, #10543:
URL: https://github.com/apache/pinot/pull/10543
Turn on v2 engine by default
--
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 #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158947857
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
datatable version is set to 4 in multistage integration tests
--
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 #10543: Turn on v2 engine by default
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158827932
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
By default data table version is 3. I think enabling multistage engine alone wouldn't be sufficient since queries will fail with data-table v3.
https://github.com/apache/pinot/blob/29fd5d9cd947f936c52bd768d45ab52088abdb30/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilderFactory.java#L37
--
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 #10543: Turn on v2 engine by default
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158827932
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
By default data table version is 3. I think enabling multistage engine alone wouldn't be sufficient since v2 queries will fail with data-table v3.
https://github.com/apache/pinot/blob/29fd5d9cd947f936c52bd768d45ab52088abdb30/pinot-core/src/main/java/org/apache/pinot/core/common/datatable/DataTableBuilderFactory.java#L37
--
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 #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1160410129
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTableImplV4.java:
##########
@@ -150,15 +150,12 @@ public DataTableImplV4(ByteBuffer byteBuffer)
}
// Read variable size data.
+ _variableSizeDataBytes = new byte[variableSizeDataLength];
Review Comment:
this should be ok.
--
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] xiangfu0 commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158310785
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineCustomTenantIntegrationTest.java:
##########
@@ -139,13 +138,11 @@ protected Connection getPinotConnection() {
@Override
protected void overrideBrokerConf(PinotConfiguration brokerConf) {
- brokerConf.setProperty(CommonConstants.Helix.CONFIG_OF_MULTI_STAGE_ENGINE_ENABLED, true);
brokerConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, 8421);
Review Comment:
The issue is instance config in helix is using port 0, so workers cannot talk to each other. Need to find the available port first then set it into server config.
--
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] xiangfu0 commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1157865245
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineCustomTenantIntegrationTest.java:
##########
@@ -139,13 +138,11 @@ protected Connection getPinotConnection() {
@Override
protected void overrideBrokerConf(PinotConfiguration brokerConf) {
- brokerConf.setProperty(CommonConstants.Helix.CONFIG_OF_MULTI_STAGE_ENGINE_ENABLED, true);
brokerConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, 8421);
Review Comment:
Those shouldn't matter, let me remove them as well
--
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 pull request #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #10543:
URL: https://github.com/apache/pinot/pull/10543#issuecomment-1498457448
test failure seems reproducible in 2 CI runs. can you take a look?
--
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 #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1157839988
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiStageEngineCustomTenantIntegrationTest.java:
##########
@@ -139,13 +138,11 @@ protected Connection getPinotConnection() {
@Override
protected void overrideBrokerConf(PinotConfiguration brokerConf) {
- brokerConf.setProperty(CommonConstants.Helix.CONFIG_OF_MULTI_STAGE_ENGINE_ENABLED, true);
brokerConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, 8421);
Review Comment:
so we still need these port config? can we check whether it is possible to default dynamic port allocation?
--
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] siddharthteotia commented on pull request #10543: Turn on v2 engine by default
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on PR #10543:
URL: https://github.com/apache/pinot/pull/10543#issuecomment-1496640530
IIRC - default enabling the feature flag will have no impact on existing production queries unless they are also changed to include the query option to use the multi stage query 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] xiangfu0 merged pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #10543:
URL: https://github.com/apache/pinot/pull/10543
--
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] siddharthteotia commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "siddharthteotia (via GitHub)" <gi...@apache.org>.
siddharthteotia commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158539072
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -170,6 +171,12 @@ public void init(PinotConfiguration serverConf)
// NOTE: Need to add the instance id to the config because it is required in HelixInstanceDataManagerConfig
_serverConf.addProperty(Server.CONFIG_OF_INSTANCE_ID, _instanceId);
}
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, QueryConfig.DEFAULT_QUERY_SERVER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, NetUtils.findOpenPort());
+ }
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, QueryConfig.DEFAULT_QUERY_RUNNER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, NetUtils.findOpenPort());
Review Comment:
Doing this can have query runner port differ from one worker to another ? In that case, how will they talk to each other ?
--
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] xiangfu0 commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158734828
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -170,6 +171,12 @@ public void init(PinotConfiguration serverConf)
// NOTE: Need to add the instance id to the config because it is required in HelixInstanceDataManagerConfig
_serverConf.addProperty(Server.CONFIG_OF_INSTANCE_ID, _instanceId);
}
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, QueryConfig.DEFAULT_QUERY_SERVER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, NetUtils.findOpenPort());
+ }
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, QueryConfig.DEFAULT_QUERY_RUNNER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, NetUtils.findOpenPort());
Review Comment:
It comes from the instanceConfig in Helix.
--
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] xiangfu0 commented on pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #10543:
URL: https://github.com/apache/pinot/pull/10543#issuecomment-1496651886
> IIRC - default enabling the feature flag will have no impact on existing production queries unless they are also changed to include the query option to use the multi stage query engine ?
This is just to start the query engine by default, ofc we will see extra resources utilization (grpc server started/ports open etc).
The normal query path won't go through it unless specifying `useMultistageEngine=true` in the query option.
--
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 #10543: Turn on v2 engine by default
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10543:
URL: https://github.com/apache/pinot/pull/10543#issuecomment-1496705394
## [Codecov](https://codecov.io/gh/apache/pinot/pull/10543?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 [#10543](https://codecov.io/gh/apache/pinot/pull/10543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a29d141) into [master](https://codecov.io/gh/apache/pinot/commit/1fc81505e4ca39e2eda9d8912e79a01e686924a6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1fc8150) will **increase** coverage by `35.35%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #10543 +/- ##
=============================================
+ Coverage 27.80% 63.15% +35.35%
- Complexity 58 6058 +6000
=============================================
Files 2087 2087
Lines 112307 112327 +20
Branches 16918 16922 +4
=============================================
+ Hits 31226 70941 +39715
+ Misses 77988 36176 -41812
- Partials 3093 5210 +2117
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `24.46% <ø> (+0.11%)` | :arrow_up: |
| integration2 | `24.14% <ø> (-0.07%)` | :arrow_down: |
| unittests1 | `67.83% <ø> (?)` | |
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/10543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/10543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `24.39% <ø> (+24.39%)` | :arrow_up: |
... and [1230 files with indirect coverage changes](https://codecov.io/gh/apache/pinot/pull/10543/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] xiangfu0 commented on pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #10543:
URL: https://github.com/apache/pinot/pull/10543#issuecomment-1499670880
Modified the V4 DataTable `_variableSizeDataBytes` to `byte[0]` instead of `null`.
--
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] xiangfu0 commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158906268
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
Good point, any idea this doesn't fail on integration test?
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
cc: @walterddr
--
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 #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158948704
##########
pinot-tools/src/main/java/org/apache/pinot/tools/MultistageEngineQuickStart.java:
##########
@@ -94,7 +94,6 @@ public String[] getDefaultBatchTableDirectories() {
@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);
Review Comment:
see: https://github.com/apache/pinot/pull/10543/files#diff-9724fd2cc0075a3c9c9a29cc7b98bb921cfe2690c2cc0b6e0ad47dec0589e6abL83
--
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] xiangfu0 commented on a diff in pull request #10543: Turn on v2 engine by default
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1160384102
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTableImplV4.java:
##########
@@ -150,15 +150,12 @@ public DataTableImplV4(ByteBuffer byteBuffer)
}
// Read variable size data.
+ _variableSizeDataBytes = new byte[variableSizeDataLength];
Review Comment:
@walterddr Please verify this logic.
--
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 #10543: Turn on v2 engine by default
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10543:
URL: https://github.com/apache/pinot/pull/10543#discussion_r1158616587
##########
pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java:
##########
@@ -170,6 +171,12 @@ public void init(PinotConfiguration serverConf)
// NOTE: Need to add the instance id to the config because it is required in HelixInstanceDataManagerConfig
_serverConf.addProperty(Server.CONFIG_OF_INSTANCE_ID, _instanceId);
}
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, QueryConfig.DEFAULT_QUERY_SERVER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_SERVER_PORT, NetUtils.findOpenPort());
+ }
+ if (_serverConf.getProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, QueryConfig.DEFAULT_QUERY_RUNNER_PORT) == 0) {
+ _serverConf.setProperty(QueryConfig.KEY_OF_QUERY_RUNNER_PORT, NetUtils.findOpenPort());
Review Comment:
port configurations are stored in serverInstance and it is backed up to Helix via InstanceConfig
--
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