Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding cluster config to config number of concurrent tasks per instance for minion task: SegmentGenerationAndPushTaskGenerator #6468

Conversation

xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Jan 20, 2021

Description

  • Adding cluster config: SegmentGenerationAndPushTask.numConcurrentTasksPerInstance to config number of concurrent tasks per instance for minion task: SegmentGenerationAndPushTaskGenerator
  • Adding new API in ClusterInfoAccessor to fetch cluster config

…TaskGenerator number of concurrent tasks per instance
@Override
public int getNumConcurrentTasksPerInstance() {
String numConcurrentTasksPerInstance = _clusterInfoAccessor
.getClusterConfig(MinionConstants.SegmentGenerationAndPushTask.CONFIG_NUMBER_CONCURRENT_TASKS_PER_INSTANCE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use static import ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I just keep the same convention as in this file. Maybe good to have another PR to swipe all the conventions ;)

@xiangfu0 xiangfu0 force-pushed the adding_cluster_config_for_minion_SegmentGenerationAndPushTaskGenerator branch 3 times, most recently from 76e299e to fd21423 Compare January 20, 2021 23:12
@xiangfu0 xiangfu0 force-pushed the adding_cluster_config_for_minion_SegmentGenerationAndPushTaskGenerator branch from fd21423 to 9122244 Compare January 21, 2021 00:17
@codecov-io
Copy link

codecov-io commented Jan 21, 2021

Codecov Report

Merging #6468 (8c81dea) into master (1beaab5) will decrease coverage by 22.62%.
The diff coverage is 38.65%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #6468       +/-   ##
===========================================
- Coverage   66.44%   43.82%   -22.63%     
===========================================
  Files        1075     1332      +257     
  Lines       54773    65318    +10545     
  Branches     8168     9522     +1354     
===========================================
- Hits        36396    28624     -7772     
- Misses      15700    34302    +18602     
+ Partials     2677     2392      -285     
Flag Coverage Δ
integration 43.82% <38.65%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ot/broker/broker/AllowAllAccessControlFactory.java 100.00% <ø> (ø)
.../helix/BrokerUserDefinedMessageHandlerFactory.java 52.83% <0.00%> (-13.84%) ⬇️
...org/apache/pinot/broker/queryquota/HitCounter.java 0.00% <0.00%> (-100.00%) ⬇️
...che/pinot/broker/queryquota/MaxHitRateTracker.java 0.00% <0.00%> (ø)
...ache/pinot/broker/queryquota/QueryQuotaEntity.java 0.00% <0.00%> (-50.00%) ⬇️
...ker/routing/instanceselector/InstanceSelector.java 100.00% <ø> (ø)
...ceselector/StrictReplicaGroupInstanceSelector.java 0.00% <0.00%> (ø)
...roker/routing/segmentpruner/TimeSegmentPruner.java 0.00% <0.00%> (ø)
...roker/routing/segmentpruner/interval/Interval.java 0.00% <0.00%> (ø)
...r/routing/segmentpruner/interval/IntervalTree.java 0.00% <0.00%> (ø)
... and 1323 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60c802c...8c81dea. Read the comment docs.

Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM otherwise

@xiangfu0 xiangfu0 merged commit f17be35 into master Jan 21, 2021
@xiangfu0 xiangfu0 deleted the adding_cluster_config_for_minion_SegmentGenerationAndPushTaskGenerator branch January 21, 2021 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants