You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "cypherean (via GitHub)" <gi...@apache.org> on 2026/01/13 18:59:42 UTC
[PR] [draft] Add warning on result truncation in MSE due to leaf limit [pinot]
cypherean opened a new pull request, #17502:
URL: https://github.com/apache/pinot/pull/17502
(no comment)
--
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
Re: [PR] [draft] Add warning on result truncation in MSE due to leaf limit [pinot]
Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #17502:
URL: https://github.com/apache/pinot/pull/17502#discussion_r2692123467
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/results/DistinctResultsBlock.java:
##########
@@ -33,6 +33,8 @@ public class DistinctResultsBlock extends BaseResultsBlock {
private final QueryContext _queryContext;
private DistinctTable _distinctTable;
+ private boolean _liteLeafLimitReached;
Review Comment:
I think adding this in individual results block will not work.
Instead can you consider tracking this in the InstanceResponseOperator? I'd recommend running a quickstart and setting breakpoints to understand which exact operators are called for the leaf stage in lite mode in the servers.
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTable.java:
##########
@@ -151,11 +151,13 @@ enum MetadataKey {
// Needed so that we can track workload name in Netty channel response.
WORKLOAD_NAME(40, "workloadName", MetadataValueType.STRING),
// Needed so that we can track query id in Netty channel response.
- QUERY_ID(41, "queryId", MetadataValueType.STRING);
+ QUERY_ID(41, "queryId", MetadataValueType.STRING),
Review Comment:
datatable is used by v1 engine so we can leave it be
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/server/ServerPlanRequestUtils.java:
##########
@@ -148,11 +148,85 @@ private static void constructPinotQueryPlan(ServerPlanRequestContext serverConte
PinotQuery pinotQuery = serverContext.getPinotQuery();
// attach leaf node limit it not set
Integer leafNodeLimit = QueryOptionsUtils.getMultiStageLeafLimit(requestMetadata);
+ int prevLimit = pinotQuery.getLimit();
pinotQuery.setLimit(leafNodeLimit != null ? leafNodeLimit : DEFAULT_LEAF_NODE_LIMIT);
+ // Tag provenance if a leaf cap (explicit or lite-mode fallback) tightened the previous limit.
+ tagLeafLimitProvenanceIfTightened(pinotQuery, requestMetadata, prevLimit);
// visit the plan and create PinotQuery and determine the leaf stage boundary PlanNode.
ServerPlanRequestVisitor.walkPlanNode(stagePlan.getRootNode(), serverContext);
}
+ /**
+ * Tags queryOptions["leafLimitProvenance"]="LITE_CAP" if either:
+ * - an explicit multiStageLeafLimit (in request metadata or query options) would tighten the previous LIMIT; or
Review Comment:
this is interesting. I suppose you want to track if results were truncated by the "multiStageLeafLimit". This will be useful even for non-lite mode queries. Correct?
--
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
Re: [PR] [draft] Add warning on result truncation in MSE due to leaf limit [pinot]
Posted by "anuragrai16 (via GitHub)" <gi...@apache.org>.
anuragrai16 commented on code in PR #17502:
URL: https://github.com/apache/pinot/pull/17502#discussion_r2703247054
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/MultiStageBrokerRequestHandler.java:
##########
@@ -522,6 +522,10 @@ private BrokerResponse query(QueryEnvironment.CompiledQuery query, long requestI
RequesterIdentity requesterIdentity, RequestContext requestContext, HttpHeaders httpHeaders, Timer timer,
boolean rlsFiltersApplied)
throws QueryException, WebApplicationException {
+ // Strip any user-supplied internal-only options to prevent spoofing. These will be set programmatically later.
+ // NOTE: 'leafLimitProvenance' is internal and must not be accepted from client input.
Review Comment:
Is this the only occurrence of internal-only options ? Can we make it a constant set that can be added to and a function that reads the set and removes it so it can be reused across.
##########
pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTable.java:
##########
@@ -151,11 +151,13 @@ enum MetadataKey {
// Needed so that we can track workload name in Netty channel response.
WORKLOAD_NAME(40, "workloadName", MetadataValueType.STRING),
// Needed so that we can track query id in Netty channel response.
- QUERY_ID(41, "queryId", MetadataValueType.STRING);
+ QUERY_ID(41, "queryId", MetadataValueType.STRING),
+ LEAF_TRUNCATION_REASON(42, "leafTruncationReason", MetadataValueType.STRING),
+ LITE_LEAF_CAP_TRUNCATION(43, "liteLeafCapTruncation", MetadataValueType.BOOLEAN);
Review Comment:
Is the limit only meant for MSE Lite ? And is the implementation different from how MSE handles default limits ?
Also, can we keep naming consistent : `LITE_LEAF_CAP_TRUNCATION` and `LITE_LEAF_TRUNCATION_REASON` if they're linked together.
--
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
Re: [PR] [draft] Add warning on result truncation in MSE due to leaf limit [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17502:
URL: https://github.com/apache/pinot/pull/17502#issuecomment-3746303198
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17502?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `128 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 33.97%. Comparing base ([`5af9015`](https://app.codecov.io/gh/apache/pinot/commit/5af901578014b2e2f928dce70d46f0352bbe2717?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`726212c`](https://app.codecov.io/gh/apache/pinot/commit/726212c131509bc032fe19887c3767468eb62741?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 56 commits behind head on master.
:white_check_mark: All tests successful. No failed tests found.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17502?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ry/runtime/plan/server/ServerPlanRequestUtils.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-query-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fquery%2Fruntime%2Fplan%2Fserver%2FServerPlanRequestUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcXVlcnktcnVudGltZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcXVlcnkvcnVudGltZS9wbGFuL3NlcnZlci9TZXJ2ZXJQbGFuUmVxdWVzdFV0aWxzLmphdmE=) | 0.00% | [41 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../core/operator/query/SelectionOrderByOperator.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fquery%2FSelectionOrderByOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9TZWxlY3Rpb25PcmRlckJ5T3BlcmF0b3IuamF2YQ==) | 0.00% | [21 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../operator/blocks/results/DistinctResultsBlock.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fblocks%2Fresults%2FDistinctResultsBlock.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvcmVzdWx0cy9EaXN0aW5jdFJlc3VsdHNCbG9jay5qYXZh) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...operator/blocks/results/SelectionResultsBlock.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fblocks%2Fresults%2FSelectionResultsBlock.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvcmVzdWx0cy9TZWxlY3Rpb25SZXN1bHRzQmxvY2suamF2YQ==) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...tor/combine/merger/DistinctResultsBlockMerger.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fcombine%2Fmerger%2FDistinctResultsBlockMerger.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL21lcmdlci9EaXN0aW5jdFJlc3VsdHNCbG9ja01lcmdlci5qYXZh) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ombine/merger/SelectionOnlyResultsBlockMerger.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fcombine%2Fmerger%2FSelectionOnlyResultsBlockMerger.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL21lcmdlci9TZWxlY3Rpb25Pbmx5UmVzdWx0c0Jsb2NrTWVyZ2VyLmphdmE=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ine/merger/SelectionOrderByResultsBlockMerger.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fcombine%2Fmerger%2FSelectionOrderByResultsBlockMerger.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL21lcmdlci9TZWxlY3Rpb25PcmRlckJ5UmVzdWx0c0Jsb2NrTWVyZ2VyLmphdmE=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/pinot/core/operator/query/DistinctOperator.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fquery%2FDistinctOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9EaXN0aW5jdE9wZXJhdG9yLmphdmE=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...common/response/broker/BrokerResponseNativeV2.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Fresponse%2Fbroker%2FBrokerResponseNativeV2.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzcG9uc2UvYnJva2VyL0Jyb2tlclJlc3BvbnNlTmF0aXZlVjIuamF2YQ==) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...a/org/apache/pinot/common/datatable/DataTable.java](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Fdatatable%2FDataTable.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZGF0YXRhYmxlL0RhdGFUYWJsZS5qYXZh) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| ... and [2 more](https://app.codecov.io/gh/apache/pinot/pull/17502?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
> :exclamation: There is a different number of reports uploaded between BASE (5af9015) and HEAD (726212c). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (5af9015) | HEAD (726212c) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|0|
>|unittests|4|1|
>|temurin|10|1|
>|java-11|5|0|
>|unittests2|2|1|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17502 +/- ##
=============================================
- Coverage 63.30% 33.97% -29.33%
+ Complexity 1474 767 -707
=============================================
Files 3157 3167 +10
Lines 188334 189282 +948
Branches 28809 28973 +164
=============================================
- Hits 119217 64305 -54912
- Misses 59898 119644 +59746
+ Partials 9219 5333 -3886
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.97% <0.00%> (-29.30%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.97% <0.00%> (-29.33%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.97% <0.00%> (-29.33%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17502/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.97% <0.00%> (-0.02%)` | :arrow_down: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/17502?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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