[draft] Add warning on result truncation in MSE due to leaf limit#17502
[draft] Add warning on result truncation in MSE due to leaf limit#17502cypherean wants to merge 3 commits intoapache:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17502 +/- ##
=============================================
- Coverage 63.30% 33.98% -29.33%
+ Complexity 1474 773 -701
=============================================
Files 3157 3167 +10
Lines 188334 189284 +950
Branches 28809 28973 +164
=============================================
- Hits 119217 64319 -54898
- Misses 59898 119634 +59736
+ Partials 9219 5331 -3888
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| 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), |
There was a problem hiding this comment.
datatable is used by v1 engine so we can leave it be
| private final QueryContext _queryContext; | ||
|
|
||
| private DistinctTable _distinctTable; | ||
| private boolean _liteLeafLimitReached; |
There was a problem hiding this 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.
|
|
||
| /** | ||
| * Tags queryOptions["leafLimitProvenance"]="LITE_CAP" if either: | ||
| * - an explicit multiStageLeafLimit (in request metadata or query options) would tighten the previous LIMIT; or |
There was a problem hiding this 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?
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
I don't follow the scope of this PR. Is there a design for this? Is this only associated with MSE lite mode?
| 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. |
There was a problem hiding this 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.
| 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); |
There was a problem hiding this 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.
5 participants
There are two cases when we add the leaflimitprovenance - when leaf limit is explicitly set in the query(for both mse and mse lite) and leaf limit in MSE lite.
Approach: