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 2021/09/29 00:20:35 UTC

[GitHub] [pinot] Jackie-Jiang opened a new pull request #7492: Add option to limit thread usage per query

Jackie-Jiang opened a new pull request #7492:
URL: https://github.com/apache/pinot/pull/7492


   ## Description
   Add instance config and query option to limit the maximum execution threads used for a query.
   By default (same as existing behavior), pinot will use up to 10 threads (or half of the CPU cores is it is smaller than 10) for non-group-by queries; and all threads for group-by queries.
   This option can be used to:
   - Limit the thread usage for very expensive queries such as large group-by or full table scan
   - Fully utilize all the threads for non-group-by queries to get the smallest latency
   
   Example run of large group-by queries (10 segments, each with 10M records, grouping on a column of cardinality 10M):
   
   Without thread limit:
   - Latency: 4103ms
   - Total thread time: 38353324527ns
   
   With thread limit 1 (single thread):
   - Latency: 10495ms
   - Total thread time: 10487257767ns
   
   Profile of CPU usage (run without limit 3 times, then single thread 3 times)
   ![Screen Shot 2021-09-28 at 5 06 56 PM](https://user-images.githubusercontent.com/17555551/135182486-111f1a83-5c5c-41ee-b009-564917df3d7a.png)
   
   ## Release Notes
   Added server instance config `pinot.server.query.executor.max.execution.threads` to configure the instance level thread limit
   Added query option `maxExecutionThreads` to configure the query level thread limit, which can be used to override the instance level limit if both exist


-- 
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] Jackie-Jiang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-931715631


   @mcvsubbu Good idea to bound the per-query limit by the instance limit. Let me add that in a separate PR


-- 
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] mcvsubbu commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-931476544


   Just going by the comments here, it may be good to impose that the per-query value of maxthreads is not above the instance config (i.e. the instance config is the max that anyone can ever set). I am not sure if this check is already being performed. @Jackie-Jiang 


-- 
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] Jackie-Jiang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929755881






-- 
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] snleee edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930443143


   @mqliang Our old behavior is to use `min(numOperator, MAX_NUM_THREADS_PER_QUERY)` for all incoming queries. So, even if the user tries to be greedy (I hope this is not the case in reality), they will get the same behavior as before. I think that we can consider enabling the `server` level config by default for our large MT cluster given that the latency requirement is not too strict. This can protect the cluster from a single super expensive query. In our current scheme, one super-expensive query can bring down the cluster. I think that it's already a good start.
   
   Yes, if we want to solve the problem of `fairness` among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem. 


-- 
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 edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293






-- 
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] mqliang edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mqliang edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930464003


   @snleee 
   >  if we want to solve the problem of fairness among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem.
   
   A proactive scheduling algo to ensure "fairness" in the first place may be a overkill, but a protective admission control is helpful to protect the whole cluster and each server.
   
   @Jackie-Jiang 
   >  but also build mechanism to limit regular users from overriding these configs.
   
   People usually call this topic Authorization(AuthZ), which is a different story from admission control.


-- 
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] snleee commented on a change in pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee commented on a change in pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#discussion_r718220702



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -55,27 +55,21 @@
   protected final QueryContext _queryContext;
   protected final ExecutorService _executorService;
   protected final long _endTimeMs;
-  protected final int _numThreads;
+  protected final int _numTasks;

Review comment:
       Can you add a comment on what the `task` would mean in this context? (e.g. the unit of parallelism)




-- 
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] snleee commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930818835


   @mqliang I think that this is a good topic for discussion. Let's create an issue to capture this. Can you elaborate more about `admission control vs authorization` in the issue? Since this PR is already closed, it will be a bit hard to continue the discussion here.


-- 
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] snleee edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930443143


   @mqliang Our old behavior is to use `min(numOperator, MAX_NUM_THREADS_PER_QUERY)` for all incoming queries. So, even if the user tries to be greedy (I hope this is not the case in reality), they will get the same behavior as before. I think that we can consider enabling the `server` level config by default for our large MT cluster given that the latency requirement is not too strict. This can protect the cluster from a single super expensive query. In our current scheme, one super-expensive query can bring down the cluster. I think that it's already a good start.
   
   Yes, if we want to solve the problem of `fairness` among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem. 


-- 
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 edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?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 [#7492](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b951928) into [master](https://codecov.io/gh/apache/pinot/commit/240bcb80546454d5ab449d3d3b13867f4943706d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240bcb8) will **increase** coverage by `39.78%`.
   > The diff coverage is `76.11%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7492/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7492       +/-   ##
   =============================================
   + Coverage     32.19%   71.98%   +39.78%     
   - Complexity        0     3412     +3412     
   =============================================
     Files          1514     1523        +9     
     Lines         75167    75529      +362     
     Branches      10966    11010       +44     
   =============================================
   + Hits          24203    54369    +30166     
   + Misses        48870    17502    -31368     
   - Partials       2094     3658     +1564     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `30.45% <65.67%> (+0.04%)` | :arrow_up: |
   | integration2 | `29.02% <70.14%> (+0.01%)` | :arrow_up: |
   | unittests1 | `69.85% <71.64%> (?)` | |
   | unittests2 | `14.49% <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://codecov.io/gh/apache/pinot/pull/7492?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/7492/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (+26.31%)` | :arrow_up: |
   | [...operator/combine/SelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9ubHlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `58.62% <20.00%> (ø)` | |
   | [...rator/combine/SelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9yZGVyQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `51.72% <33.33%> (ø)` | |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `74.40% <50.00%> (ø)` | |
   | [.../java/org/apache/pinot/core/util/QueryOptions.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1F1ZXJ5T3B0aW9ucy5qYXZh) | `90.32% <50.00%> (+24.80%)` | :arrow_up: |
   | [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `82.53% <81.81%> (+15.58%)` | :arrow_up: |
   | [...va/org/apache/pinot/core/plan/CombinePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0NvbWJpbmVQbGFuTm9kZS5qYXZh) | `88.52% <88.88%> (+3.52%)` | :arrow_up: |
   | [...erator/combine/AggregationOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0FnZ3JlZ2F0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `75.00% <100.00%> (ø)` | |
   | [...not/core/operator/combine/BaseCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0Jhc2VDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `92.75% <100.00%> (+3.71%)` | :arrow_up: |
   | [...ot/core/operator/combine/CombineOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0NvbWJpbmVPcGVyYXRvclV0aWxzLmphdmE=) | `100.00% <100.00%> (ø)` | |
   | ... and [1010 more](https://codecov.io/gh/apache/pinot/pull/7492/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [240bcb8...b951928](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] Jackie-Jiang merged pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7492:
URL: https://github.com/apache/pinot/pull/7492


   


-- 
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 edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?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 [#7492](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b951928) into [master](https://codecov.io/gh/apache/pinot/commit/240bcb80546454d5ab449d3d3b13867f4943706d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240bcb8) will **increase** coverage by `38.55%`.
   > The diff coverage is `76.11%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7492/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7492       +/-   ##
   =============================================
   + Coverage     32.19%   70.74%   +38.55%     
   - Complexity        0     3412     +3412     
   =============================================
     Files          1514     1523        +9     
     Lines         75167    75529      +362     
     Branches      10966    11010       +44     
   =============================================
   + Hits          24203    53436    +29233     
   + Misses        48870    18444    -30426     
   - Partials       2094     3649     +1555     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `29.02% <70.14%> (+0.01%)` | :arrow_up: |
   | unittests1 | `69.85% <71.64%> (?)` | |
   | unittests2 | `14.49% <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://codecov.io/gh/apache/pinot/pull/7492?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/7492/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (+26.31%)` | :arrow_up: |
   | [...operator/combine/SelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9ubHlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `58.62% <20.00%> (ø)` | |
   | [...rator/combine/SelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9yZGVyQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `51.72% <33.33%> (ø)` | |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `74.40% <50.00%> (ø)` | |
   | [.../java/org/apache/pinot/core/util/QueryOptions.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1F1ZXJ5T3B0aW9ucy5qYXZh) | `90.32% <50.00%> (+24.80%)` | :arrow_up: |
   | [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `82.53% <81.81%> (+15.58%)` | :arrow_up: |
   | [...va/org/apache/pinot/core/plan/CombinePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0NvbWJpbmVQbGFuTm9kZS5qYXZh) | `88.52% <88.88%> (+3.52%)` | :arrow_up: |
   | [...erator/combine/AggregationOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0FnZ3JlZ2F0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `75.00% <100.00%> (ø)` | |
   | [...not/core/operator/combine/BaseCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0Jhc2VDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `82.60% <100.00%> (-6.44%)` | :arrow_down: |
   | [...ot/core/operator/combine/CombineOperatorUtils.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0NvbWJpbmVPcGVyYXRvclV0aWxzLmphdmE=) | `100.00% <100.00%> (ø)` | |
   | ... and [1061 more](https://codecov.io/gh/apache/pinot/pull/7492/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [240bcb8...b951928](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] Jackie-Jiang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929755881


   @xiangfu0 Good point. This is independent of this PR, so added #7494 to track that


-- 
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] snleee edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930818835


   @mqliang I think that this is a good topic for discussion. Let's create an issue to capture this. Can you add a brief summary of the `admission control vs authorization` approaches? I'm curious about how you would approach protecting the cluster given a query string. Since this PR is already closed, it will be a bit hard to continue the discussion here.


-- 
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] mqliang edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mqliang edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930464003


   @snleee 
   >  if we want to solve the problem of fairness among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem.
   
   A proactive scheduling algo to ensure "fairness" in the first place may be a overkill, but a protective admission control is helpful to protect the whole cluster and each server.
   
   @Jackie-Jiang 
   >  but also build mechanism to limit regular users from overriding these configs.
   
   People usually call this topic Authorization(AuthZ), which is a different story from admission control.


-- 
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 #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929738500


   This is awesome!
   
   One high level question, shall we consider set this numExecutionThreads part of QueryContext?


-- 
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 #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?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 [#7492](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b951928) into [master](https://codecov.io/gh/apache/pinot/commit/240bcb80546454d5ab449d3d3b13867f4943706d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240bcb8) will **increase** coverage by `37.65%`.
   > The diff coverage is `71.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7492/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7492       +/-   ##
   =============================================
   + Coverage     32.19%   69.85%   +37.65%     
   - Complexity        0     3332     +3332     
   =============================================
     Files          1514     1129      -385     
     Lines         75167    53424    -21743     
     Branches      10966     8049     -2917     
   =============================================
   + Hits          24203    37320    +13117     
   + Misses        48870    13455    -35415     
   - Partials       2094     2649      +555     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.85% <71.64%> (?)` | |
   
   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/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...reaming/StreamingSelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nU2VsZWN0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (-70.46%)` | :arrow_down: |
   | [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (+26.31%)` | :arrow_up: |
   | [...operator/combine/SelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9ubHlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `58.62% <20.00%> (ø)` | |
   | [...rator/combine/SelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9yZGVyQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `24.13% <33.33%> (-27.59%)` | :arrow_down: |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `62.40% <50.00%> (-12.01%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/QueryOptions.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1F1ZXJ5T3B0aW9ucy5qYXZh) | `80.64% <50.00%> (+15.12%)` | :arrow_up: |
   | [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `72.22% <72.72%> (+5.26%)` | :arrow_up: |
   | [...va/org/apache/pinot/core/plan/CombinePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0NvbWJpbmVQbGFuTm9kZS5qYXZh) | `85.24% <88.88%> (+0.24%)` | :arrow_up: |
   | [...erator/combine/AggregationOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0FnZ3JlZ2F0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `75.00% <100.00%> (ø)` | |
   | [...not/core/operator/combine/BaseCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0Jhc2VDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `82.60% <100.00%> (-6.44%)` | :arrow_down: |
   | ... and [1316 more](https://codecov.io/gh/apache/pinot/pull/7492/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [240bcb8...b951928](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov-commenter edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?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 [#7492](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b951928) into [master](https://codecov.io/gh/apache/pinot/commit/240bcb80546454d5ab449d3d3b13867f4943706d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240bcb8) will **increase** coverage by `32.95%`.
   > The diff coverage is `71.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7492/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7492       +/-   ##
   =============================================
   + Coverage     32.19%   65.15%   +32.95%     
   - Complexity        0     3412     +3412     
   =============================================
     Files          1514     1477       -37     
     Lines         75167    73679     -1488     
     Branches      10966    10810      -156     
   =============================================
   + Hits          24203    48002    +23799     
   + Misses        48870    22262    -26608     
   - Partials       2094     3415     +1321     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.85% <71.64%> (?)` | |
   | unittests2 | `14.49% <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://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...reaming/StreamingSelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nU2VsZWN0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (-70.46%)` | :arrow_down: |
   | [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (+26.31%)` | :arrow_up: |
   | [...operator/combine/SelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9ubHlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `58.62% <20.00%> (ø)` | |
   | [...rator/combine/SelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9yZGVyQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `24.13% <33.33%> (-27.59%)` | :arrow_down: |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `62.40% <50.00%> (-12.01%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/QueryOptions.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1F1ZXJ5T3B0aW9ucy5qYXZh) | `80.64% <50.00%> (+15.12%)` | :arrow_up: |
   | [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `72.22% <72.72%> (+5.26%)` | :arrow_up: |
   | [...va/org/apache/pinot/core/plan/CombinePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0NvbWJpbmVQbGFuTm9kZS5qYXZh) | `85.24% <88.88%> (+0.24%)` | :arrow_up: |
   | [...erator/combine/AggregationOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0FnZ3JlZ2F0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `75.00% <100.00%> (ø)` | |
   | [...not/core/operator/combine/BaseCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0Jhc2VDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `82.60% <100.00%> (-6.44%)` | :arrow_down: |
   | ... and [1228 more](https://codecov.io/gh/apache/pinot/pull/7492/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [240bcb8...b951928](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov-commenter commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929737293


   # [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?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 [#7492](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b951928) into [master](https://codecov.io/gh/apache/pinot/commit/240bcb80546454d5ab449d3d3b13867f4943706d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (240bcb8) will **increase** coverage by `37.65%`.
   > The diff coverage is `71.64%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/pinot/pull/7492/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #7492       +/-   ##
   =============================================
   + Coverage     32.19%   69.85%   +37.65%     
   - Complexity        0     3332     +3332     
   =============================================
     Files          1514     1129      -385     
     Lines         75167    53424    -21743     
     Branches      10966     8049     -2917     
   =============================================
   + Hits          24203    37320    +13117     
   + Misses        48870    13455    -35415     
   - Partials       2094     2649      +555     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `69.85% <71.64%> (?)` | |
   
   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/7492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...reaming/StreamingSelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9zdHJlYW1pbmcvU3RyZWFtaW5nU2VsZWN0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (-70.46%)` | :arrow_down: |
   | [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `26.31% <ø> (+26.31%)` | :arrow_up: |
   | [...operator/combine/SelectionOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9ubHlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `58.62% <20.00%> (ø)` | |
   | [...rator/combine/SelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL1NlbGVjdGlvbk9yZGVyQnlDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `24.13% <33.33%> (-27.59%)` | :arrow_down: |
   | [...nMaxValueBasedSelectionOrderByCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL01pbk1heFZhbHVlQmFzZWRTZWxlY3Rpb25PcmRlckJ5Q29tYmluZU9wZXJhdG9yLmphdmE=) | `62.40% <50.00%> (-12.01%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/QueryOptions.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1F1ZXJ5T3B0aW9ucy5qYXZh) | `80.64% <50.00%> (+15.12%)` | :arrow_up: |
   | [...pinot/core/plan/maker/InstancePlanMakerImplV2.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL21ha2VyL0luc3RhbmNlUGxhbk1ha2VySW1wbFYyLmphdmE=) | `72.22% <72.72%> (+5.26%)` | :arrow_up: |
   | [...va/org/apache/pinot/core/plan/CombinePlanNode.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0NvbWJpbmVQbGFuTm9kZS5qYXZh) | `85.24% <88.88%> (+0.24%)` | :arrow_up: |
   | [...erator/combine/AggregationOnlyCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0FnZ3JlZ2F0aW9uT25seUNvbWJpbmVPcGVyYXRvci5qYXZh) | `75.00% <100.00%> (ø)` | |
   | [...not/core/operator/combine/BaseCombineOperator.java](https://codecov.io/gh/apache/pinot/pull/7492/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9jb21iaW5lL0Jhc2VDb21iaW5lT3BlcmF0b3IuamF2YQ==) | `82.60% <100.00%> (-6.44%)` | :arrow_down: |
   | ... and [1316 more](https://codecov.io/gh/apache/pinot/pull/7492/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) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [240bcb8...b951928](https://codecov.io/gh/apache/pinot/pull/7492?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] mqliang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930464003


   @snleee 
   >  if we want to solve the problem of fairness among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem.
   
   A proactive scheduling algo to ensure "fairness" in the first place may be a overkill, but a protective admission control is helpful to protect the whole cluster and each server.
   
   @Jackie-Jiang 
   >  but also build mechanism to limit regular users from overriding these configs.
   
   This is a Authorization(AuthZ), which is a different story from admission control.


-- 
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] mqliang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930419711






-- 
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] Jackie-Jiang commented on a change in pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#discussion_r718761435



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -55,27 +55,21 @@
   protected final QueryContext _queryContext;
   protected final ExecutorService _executorService;
   protected final long _endTimeMs;
-  protected final int _numThreads;
+  protected final int _numTasks;

Review comment:
       Added




-- 
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] Jackie-Jiang commented on a change in pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#discussion_r718761435



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -55,27 +55,21 @@
   protected final QueryContext _queryContext;
   protected final ExecutorService _executorService;
   protected final long _endTimeMs;
-  protected final int _numThreads;
+  protected final int _numTasks;

Review comment:
       Added




-- 
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] mqliang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930419711


   @snleee @Jackie-Jiang 
   > I think that this is an interesting feature for the multi-tenant cluster.
   
   Do we have admission control mechanism? In multi-tenant environment, if we don't have good admission controlling, a greedy user will set the value of query level option "maxExecutionThreads" as an extremely large value to get the best performance (even if they don't need that), and impact other users. What made matters worse was that, once other users aware that heir performance drops due to a greedy user “stealing” the public resource, they will also increase their config value. Which will eventually burn out the server. 


-- 
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 #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-929738500


   This is awesome!
   
   One high level question, shall we consider set this numExecutionThreads part of QueryContext?


-- 
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] snleee commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930443143


   @mqliang Our old behavior is to use `min(numOperator, MAX_NUM_THREADS_PER_QUERY)` for all incoming queries. So, even if the user tries to be greedy (I hope this is not the case in reality), they will get the same behavior as before. I think that we can consider enabling the `server` level config by default for our large MT cluster given that the latency requirement is not too strict. This can protect the cluster from a single super expensive query. In our current scheme, one super-expensive query can bring down the cluster. I think that it's already a good start.
   
   Yes, if we want to solve the problem of "fairness" among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem. 


-- 
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] snleee edited a comment on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee edited a comment on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930818835


   @mqliang I think that this is a good topic for discussion. Let's create an issue to capture this. Can you add a brief summary of the `admission control vs authorization` approaches? I'm curious about how you would protect the cluster given a query string. Since this PR is already closed, it will be a bit hard to continue the discussion here.


-- 
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] Jackie-Jiang merged pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7492:
URL: https://github.com/apache/pinot/pull/7492


   


-- 
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] Jackie-Jiang commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930445994


   @mqliang Good point. I considered that and we want to add admission control so that regular users cannot set the query options. This PR won't make things worse because the group-by queries (most expensive ones) are already using all the system resources.
   We should build all the knobs so that admin can control how the query should be executed, but also build mechanism to limit regular users from overriding these configs.


-- 
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] snleee commented on pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee commented on pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#issuecomment-930443143


   @mqliang Our old behavior is to use `min(numOperator, MAX_NUM_THREADS_PER_QUERY)` for all incoming queries. So, even if the user tries to be greedy (I hope this is not the case in reality), they will get the same behavior as before. I think that we can consider enabling the `server` level config by default for our large MT cluster given that the latency requirement is not too strict. This can protect the cluster from a single super expensive query. In our current scheme, one super-expensive query can bring down the cluster. I think that it's already a good start.
   
   Yes, if we want to solve the problem of "fairness" among all clients for our MT cluster resource, a more sophisticated scheduling algorithm will be needed and this would be a complex research problem. 


-- 
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] snleee commented on a change in pull request #7492: Add option to limit thread usage per query

Posted by GitBox <gi...@apache.org>.
snleee commented on a change in pull request #7492:
URL: https://github.com/apache/pinot/pull/7492#discussion_r718220702



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/combine/BaseCombineOperator.java
##########
@@ -55,27 +55,21 @@
   protected final QueryContext _queryContext;
   protected final ExecutorService _executorService;
   protected final long _endTimeMs;
-  protected final int _numThreads;
+  protected final int _numTasks;

Review comment:
       Can you add a comment on what the `task` would mean in this context? (e.g. the unit of parallelism)




-- 
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