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/09/11 17:48:53 UTC
[GitHub] [pinot] walterddr opened a new pull request, #9379: [multistage] hybrid routing support
walterddr opened a new pull request, #9379:
URL: https://github.com/apache/pinot/pull/9379
Overview
===
This PR is preliminary support for hybrid routing on multistage engine. It copies so of the logics in the broker request side but not all. and only has basic testing.
Details
===
- copied some time boundary logics (manager and associated logics) into either core module or replicated in multi-stage planner
- created a hybrid routing dispatchable stagemetadata
- indexed each segment with table types
- added time boundary info if dispatching multiple table types, otherwise ignore
- reconstructed server requests multiple times.
--
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] kishoreg commented on a diff in pull request #9379: [multistage] hybrid routing support
Posted by GitBox <gi...@apache.org>.
kishoreg commented on code in PR #9379:
URL: https://github.com/apache/pinot/pull/9379#discussion_r967874234
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -58,11 +60,37 @@ public WorkerManager(String hostName, int port, RoutingManager routingManager) {
public void assignWorkerToStage(int stageId, StageMetadata stageMetadata) {
List<String> scannedTables = stageMetadata.getScannedTables();
- if (scannedTables.size() == 1) { // table scan stage, need to attach server as well as segment info.
- RoutingTable routingTable = getRoutingTable(scannedTables.get(0));
- Map<ServerInstance, List<String>> serverInstanceToSegmentsMap = routingTable.getServerInstanceToSegmentsMap();
+ if (scannedTables.size() == 1) {
+ // table scan stage, need to attach server as well as segment info for each physical table type.
+ Map<String, RoutingTable> routingTableMap = getRoutingTable(scannedTables.get(0));
Review Comment:
is scannedtable referring to logical table name or tablename with type.. can we name variable to indicate that?
##########
pinot-common/src/main/proto/worker.proto:
##########
@@ -68,7 +68,13 @@ message StagePlan {
message StageMetadata {
repeated string instances = 1;
repeated string dataSources = 2;
- map<string, SegmentList> instanceToSegmentList = 3;
+ map<string, SegmentMap> instanceToSegmentMap = 3;
Review Comment:
we can add another map<string,SegmentList> right? one for real time and one for offline.. thought your approah is more flexible if we add more tabletype
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/routing/WorkerManager.java:
##########
@@ -86,13 +114,23 @@ private static List<ServerInstance> filterServers(Collection<ServerInstance> ser
return serverInstances;
}
- private RoutingTable getRoutingTable(String tableName) {
+ private Map<String, RoutingTable> getRoutingTable(String tableName) {
Review Comment:
tableName -> tableNameWithType right?
--
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 #9379: [multistage] hybrid routing support
Posted by GitBox <gi...@apache.org>.
walterddr merged PR #9379:
URL: https://github.com/apache/pinot/pull/9379
--
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 #9379: [multistage] hybrid routing support
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9379:
URL: https://github.com/apache/pinot/pull/9379#discussion_r967872452
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/QueryRunner.java:
##########
@@ -19,8 +19,10 @@
package org.apache.pinot.query.runtime;
import java.nio.ByteBuffer;
+import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
Review Comment:
```suggestion
```
--
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 #9379: [multistage] hybrid routing support
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #9379:
URL: https://github.com/apache/pinot/pull/9379#issuecomment-1243017810
# [Codecov](https://codecov.io/gh/apache/pinot/pull/9379?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 [#9379](https://codecov.io/gh/apache/pinot/pull/9379?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (327ab44) into [master](https://codecov.io/gh/apache/pinot/commit/65e97d23efaa3afc5b6af653575b5fe5bd26756d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (65e97d2) will **decrease** coverage by `54.38%`.
> The diff coverage is `85.93%`.
```diff
@@ Coverage Diff @@
## master #9379 +/- ##
=============================================
- Coverage 69.76% 15.37% -54.39%
+ Complexity 4787 170 -4617
=============================================
Files 1885 1832 -53
Lines 100293 98112 -2181
Branches 15256 15017 -239
=============================================
- Hits 69966 15082 -54884
- Misses 25381 81886 +56505
+ Partials 4946 1144 -3802
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `15.37% <85.93%> (+0.05%)` | :arrow_up: |
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/9379?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <ø> (-85.72%)` | :arrow_down: |
| [...roker/requesthandler/BaseBrokerRequestHandler.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvQmFzZUJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `45.05% <ø> (-25.20%)` | :arrow_down: |
| [...che/pinot/broker/routing/BrokerRoutingManager.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Ccm9rZXJSb3V0aW5nTWFuYWdlci5qYXZh) | `59.60% <ø> (-26.28%)` | :arrow_down: |
| [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `91.35% <ø> (ø)` | |
| [...rg/apache/pinot/core/routing/TimeBoundaryInfo.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1RpbWVCb3VuZGFyeUluZm8uamF2YQ==) | `0.00% <ø> (ø)` | |
| [.../query/runtime/plan/serde/QueryPlanSerDeUtils.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL3NlcmRlL1F1ZXJ5UGxhblNlckRlVXRpbHMuamF2YQ==) | `90.16% <72.72%> (-9.84%)` | :arrow_down: |
| [.../pinot/query/runtime/utils/ServerRequestUtils.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS91dGlscy9TZXJ2ZXJSZXF1ZXN0VXRpbHMuamF2YQ==) | `79.31% <82.05%> (-1.05%)` | :arrow_down: |
| [...va/org/apache/pinot/query/runtime/QueryRunner.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9RdWVyeVJ1bm5lci5qYXZh) | `86.66% <87.87%> (-0.84%)` | :arrow_down: |
| [.../org/apache/pinot/query/routing/WorkerManager.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcm91dGluZy9Xb3JrZXJNYW5hZ2VyLmphdmE=) | `90.56% <96.66%> (+3.89%)` | :arrow_up: |
| [.../org/apache/pinot/query/planner/StageMetadata.java](https://codecov.io/gh/apache/pinot/pull/9379/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-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9TdGFnZU1ldGFkYXRhLmphdmE=) | `100.00% <100.00%> (ø)` | |
| ... and [1429 more](https://codecov.io/gh/apache/pinot/pull/9379/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) | |
: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] walterddr commented on a diff in pull request #9379: [multistage] hybrid routing support
Posted by GitBox <gi...@apache.org>.
walterddr commented on code in PR #9379:
URL: https://github.com/apache/pinot/pull/9379#discussion_r967897015
##########
pinot-common/src/main/proto/worker.proto:
##########
@@ -68,7 +68,13 @@ message StagePlan {
message StageMetadata {
repeated string instances = 1;
repeated string dataSources = 2;
- map<string, SegmentList> instanceToSegmentList = 3;
+ map<string, SegmentMap> instanceToSegmentMap = 3;
Review Comment:
yes this way it is more flexible
--
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