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/05/11 09:21:55 UTC
[GitHub] [pinot] xiangfu0 opened a new pull request, #10756: Refactor StageMetadata to pinot-query-runtime package
xiangfu0 opened a new pull request, #10756:
URL: https://github.com/apache/pinot/pull/10756
- Refactor StageMetadata from `pinot-query-planner` to `pinot-query-runtime` package
- Only set necessary `WorkerMetadata` for `OpChainExecutionContext`
--
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 #10756: Refactor StageMetadata to pinot-query-runtime package
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10756:
URL: https://github.com/apache/pinot/pull/10756#issuecomment-1543703806
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10756?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 [#10756](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (aacac59) into [master](https://app.codecov.io/gh/apache/pinot/commit/fe98bb0783213504c77be397b1750de3962000ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe98bb0) will **decrease** coverage by `13.75%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #10756 +/- ##
=============================================
- Coverage 27.48% 13.74% -13.75%
- Complexity 58 439 +381
=============================================
Files 2137 2099 -38
Lines 114963 112909 -2054
Branches 17313 17071 -242
=============================================
- Hits 31597 15516 -16081
- Misses 80235 96120 +15885
+ Partials 3131 1273 -1858
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests2 | `13.74% <0.00%> (?)` | |
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://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/query/planner/DispatchablePlanFragment.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcGxhbm5lci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcGxhbm5lci9EaXNwYXRjaGFibGVQbGFuRnJhZ21lbnQuamF2YQ==) | `0.00% <ø> (ø)` | |
| [...va/org/apache/pinot/query/runtime/QueryRunner.java](https://app.codecov.io/gh/apache/pinot/pull/10756?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) | `0.00% <0.00%> (ø)` | |
| [...y/runtime/operator/BaseMailboxReceiveOperator.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9CYXNlTWFpbGJveFJlY2VpdmVPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ot/query/runtime/operator/MailboxSendOperator.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9vcGVyYXRvci9NYWlsYm94U2VuZE9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...pinot/query/runtime/plan/DistributedStagePlan.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL0Rpc3RyaWJ1dGVkU3RhZ2VQbGFuLmphdmE=) | `0.00% <ø> (ø)` | |
| [...ot/query/runtime/plan/OpChainExecutionContext.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL09wQ2hhaW5FeGVjdXRpb25Db250ZXh0LmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...e/pinot/query/runtime/plan/PlanRequestContext.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL1BsYW5SZXF1ZXN0Q29udGV4dC5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...t/query/runtime/plan/ServerRequestPlanVisitor.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL1NlcnZlclJlcXVlc3RQbGFuVmlzaXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...apache/pinot/query/runtime/plan/StageMetadata.java](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL1N0YWdlTWV0YWRhdGEuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../query/runtime/plan/serde/QueryPlanSerDeUtils.java](https://app.codecov.io/gh/apache/pinot/pull/10756?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==) | `0.00% <ø> (ø)` | |
| ... and [2 more](https://app.codecov.io/gh/apache/pinot/pull/10756?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
... and [990 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10756/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] walterddr closed pull request #10756: Refactor usage of WorkerMetadata and StageMetadata
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr closed pull request #10756: Refactor usage of WorkerMetadata and StageMetadata
URL: https://github.com/apache/pinot/pull/10756
--
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 #10756: Refactor usage of WorkerMetadata and StageMetadata
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10756:
URL: https://github.com/apache/pinot/pull/10756#discussion_r1195433083
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -212,7 +212,8 @@ public static DistributedStagePlan constructDistributedStagePlan(DispatchableSub
int stageId, VirtualServerAddress serverAddress) {
return new DistributedStagePlan(stageId, serverAddress,
dispatchableSubPlan.getQueryStageList().get(stageId).getPlanFragment().getFragmentRoot(),
- dispatchableSubPlan.getQueryStageList().get(stageId).toStageMetadata());
+ StageMetadata.from(dispatchableSubPlan.getQueryStageList().get(stageId)),
Review Comment:
e.g. intead of doing this in Dispatcher
```
_executorService.submit(() -> client.submit(Worker.QueryRequest.newBuilder().setStagePlan(
QueryPlanSerDeUtils.serialize(
constructDistributedStagePlan(dispatchableSubPlan, finalStageId, virtualServerAddress)))
.putMetadata(QueryConfig.KEY_OF_BROKER_REQUEST_ID, String.valueOf(requestId))
.putMetadata(QueryConfig.KEY_OF_BROKER_REQUEST_TIMEOUT_MS, String.valueOf(timeoutMs))
.putAllMetadata(queryOptions).build(), finalStageId, queryServerInstance, deadline,
dispatchCallbacks::offer));
```
1. directly serialized the dispatchableSubPlan to proto format:
```
QueryPlanSerDeUtils.serialize(dispatchableSubPlan, finalStageId, virtualServerAddress)
```
2. when deserialize, deserialize it into multiple distributed plan
```
List<DistributedStagePlan> QueryPlanSerDeUtils.deserialize(Worker.StagerPlan stagePlan)
```
##########
pinot-common/src/main/proto/worker.proto:
##########
@@ -73,10 +73,10 @@ message StagePlan {
string virtualAddress = 2;
StageNode stageRoot = 3;
StageMetadata stageMetadata = 4;
+ WorkerMetadata workerMetadata = 5;
Review Comment:
this might not be the right abstraction for proto.
we want to send a single proto request from broker to server to launch all workers, not sending 1 grpc request per worker which will be inefficient. let's still keep the wire protocol as a list.
--
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 #10756: Refactor usage of WorkerMetadata and StageMetadata
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #10756:
URL: https://github.com/apache/pinot/pull/10756#discussion_r1195430392
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/service/dispatch/QueryDispatcher.java:
##########
@@ -212,7 +212,8 @@ public static DistributedStagePlan constructDistributedStagePlan(DispatchableSub
int stageId, VirtualServerAddress serverAddress) {
return new DistributedStagePlan(stageId, serverAddress,
dispatchableSubPlan.getQueryStageList().get(stageId).getPlanFragment().getFragmentRoot(),
- dispatchableSubPlan.getQueryStageList().get(stageId).toStageMetadata());
+ StageMetadata.from(dispatchableSubPlan.getQueryStageList().get(stageId)),
Review Comment:
the right way to solve this is do not construct `DistributedStagePlan` on broker. instead construct the proto object directly and when deserialize, put them into multiple `DistributedStagePlan`
--
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 #10756: Refactor usage of WorkerMetadata and StageMetadata
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on PR #10756:
URL: https://github.com/apache/pinot/pull/10756#issuecomment-1572946323
this can be closed as covered by https://github.com/apache/pinot/pull/10791
--
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