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/06/28 22:02:28 UTC
[GitHub] [incubator-pinot] mqliang opened a new pull request #7099: Add minion metrics of task queueing time and task numbers
mqliang opened a new pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099
## Description
<!-- Add a description of your PR here.
A good description should include pointers to an issue or design document, etc.
-->
This PR add minion metric/Gauge of:
* task queuing time: task_dequeue_time - task_inqueue_time (assume the time drift between helix controller and pinot minion is not large, otherwise the value may be negative)
* number of tasks in progress. Whenever minion start a task, increase the Gauge by 1, whenever minion completed (either succeeded or failed) a task, decrease it by 1.
cc @mcvsubbu @jackjlli
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660989212
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,19 +71,31 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
Review comment:
done
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -88,24 +104,24 @@ public TaskResult run() {
Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);
long timeSpentInNanos = System.nanoTime() - startTimeInNanos;
_eventObserver.notifyTaskSuccess(pinotTaskConfig, executionResult);
- minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
- minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
+ _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
Review comment:
done
--
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] [incubator-pinot] codecov-commenter commented on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8457967) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **decrease** coverage by `31.87%`.
> The diff coverage is `52.42%`.
> :exclamation: Current head 8457967 differs from pull request most recent head 22e9c22. Consider uploading reports for the commit 22e9c22 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
=============================================
- Coverage 73.49% 41.61% -31.88%
+ Complexity 91 7 -84
=============================================
Files 1492 1493 +1
Lines 73433 73531 +98
Branches 10577 10590 +13
=============================================
- Hits 53966 30599 -23367
- Misses 15938 40345 +24407
+ Partials 3529 2587 -942
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.61% <52.42%> (+0.02%)` | :arrow_up: |
| unittests | `?` | |
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/incubator-pinot/pull/7099?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/controller/ControllerConf.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `44.64% <0.00%> (-8.54%)` | :arrow_down: |
| [...ntroller/helix/core/minion/TaskMetricsEmitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrTWV0cmljc0VtaXR0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `79.41% <74.00%> (-3.55%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `97.22% <100.00%> (+0.55%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `27.77% <100.00%> (-0.27%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <100.00%> (+78.88%)` | :arrow_up: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `77.41% <100.00%> (+2.87%)` | :arrow_up: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [954 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...22e9c22](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660966564
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -88,24 +104,24 @@ public TaskResult run() {
Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);
long timeSpentInNanos = System.nanoTime() - startTimeInNanos;
_eventObserver.notifyTaskSuccess(pinotTaskConfig, executionResult);
- minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
- minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
+ _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
Review comment:
For now, runInternal() seems to catch all exceptions, yes. And we can leave it like that.
In the parent run() method, I suggest:
try {
runInternal()
} finally {
update metrics
}
This will protect against any changes inside the runInternal logic that may choose to bubble up exceptions or if there is some uncaught exception by mistake.
--
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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660963823
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -88,24 +104,24 @@ public TaskResult run() {
Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);
long timeSpentInNanos = System.nanoTime() - startTimeInNanos;
_eventObserver.notifyTaskSuccess(pinotTaskConfig, executionResult);
- minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
- minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
+ _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
Review comment:
@mcvsubbu I am a little bit confused. Currently `runInternal()` just simply call `Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);` then try/catch all kinds of exceptions, and log/emit metric accordingly. If we add try/catch in `run()` and move the log/metric logic to the `run()` method, then it's equivalent to moving all logic to`run()`:
```java
public TaskResult run() {
try {
Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);
// log and emit success message/metric
return new TaskResult(TaskResult.Status.COMPLETED, "Succeeded");
} catch (TaskCancelledException e) {
// log and emit cancel message/metric
return new TaskResult(TaskResult.Status.CANCELED, e.toString());
} catch (FatalException e) {
// log and emit fatal error message/metric
return new TaskResult(TaskResult.Status.FATAL_FAILED, e.toString());
} catch (Exception e) {
// log and emit error message/metric
return new TaskResult(TaskResult.Status.FAILED, e.toString());
} finally {
_minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, -1L);
}
}
private Object runInternal(PinotTaskConfig pinotTaskConfig)
throws Exception {
return _taskExecutor.executeTask(pinotTaskConfig)
}
```
--
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] [incubator-pinot] codecov-commenter commented on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8457967) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **decrease** coverage by `31.87%`.
> The diff coverage is `52.42%`.
> :exclamation: Current head 8457967 differs from pull request most recent head 22e9c22. Consider uploading reports for the commit 22e9c22 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
=============================================
- Coverage 73.49% 41.61% -31.88%
+ Complexity 91 7 -84
=============================================
Files 1492 1493 +1
Lines 73433 73531 +98
Branches 10577 10590 +13
=============================================
- Hits 53966 30599 -23367
- Misses 15938 40345 +24407
+ Partials 3529 2587 -942
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.61% <52.42%> (+0.02%)` | :arrow_up: |
| unittests | `?` | |
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/incubator-pinot/pull/7099?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/controller/ControllerConf.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `44.64% <0.00%> (-8.54%)` | :arrow_down: |
| [...ntroller/helix/core/minion/TaskMetricsEmitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrTWV0cmljc0VtaXR0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `79.41% <74.00%> (-3.55%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `97.22% <100.00%> (+0.55%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `27.77% <100.00%> (-0.27%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <100.00%> (+78.88%)` | :arrow_up: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `77.41% <100.00%> (+2.87%)` | :arrow_up: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [954 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...22e9c22](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] Jackie-Jiang commented on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870850272
Thanks for adding these metrics. After the PR is merged, can you please help update the pinot doc regarding the task metrics? It can be added in this page: https://docs.pinot.apache.org/basics/components/minion
--
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660152383
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -108,6 +120,8 @@ public TaskResult run() {
minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FAILED, 1L);
LOGGER.error("Caught exception while executing task: {}", _taskConfig.getId(), e);
return new TaskResult(TaskResult.Status.FAILED, e.toString());
+ } finally {
+ minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, -1L);
Review comment:
Best to call a method `runInternal` that has the current logic, and manipulate this metric before and after the call outside (in the run() method). That way, if someone adds a return from an earlier point, we don't have to worry about the metric going bad.
--
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] [incubator-pinot] jackjlli commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660968992
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,19 +71,31 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
Review comment:
One more thing to note is that `MinionQueryPhase.TASK_EXECUTION` logs time in **nanoseconds** while this one logs time in **milliseconds**. The time granularity needs to be unified.
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db76bac) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **decrease** coverage by `8.06%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
============================================
- Coverage 73.49% 65.42% -8.07%
Complexity 91 91
============================================
Files 1492 1495 +3
Lines 73433 73586 +153
Branches 10577 10595 +18
============================================
- Hits 53966 48146 -5820
- Misses 15938 22038 +6100
+ Partials 3529 3402 -127
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.42% <0.00%> (-0.09%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `0.00% <0.00%> (-11.12%)` | :arrow_down: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `0.00% <0.00%> (-74.55%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [355 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...db76bac](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660850504
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -88,24 +104,24 @@ public TaskResult run() {
Object executionResult = _taskExecutor.executeTask(pinotTaskConfig);
long timeSpentInNanos = System.nanoTime() - startTimeInNanos;
_eventObserver.notifyTaskSuccess(pinotTaskConfig, executionResult);
- minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
- minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
+ _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_COMPLETED, 1L);
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_EXECUTION, timeSpentInNanos);
Review comment:
move this to the run() method
--
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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r661000329
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,60 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ try {
+ _minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
+ long startTimeMillis = System.currentTimeMillis();
+ TaskResult result = runInternal();
+ long timeSpentInMillis = System.currentTimeMillis() - startTimeMillis;
Review comment:
done
--
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660152383
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -108,6 +120,8 @@ public TaskResult run() {
minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FAILED, 1L);
LOGGER.error("Caught exception while executing task: {}", _taskConfig.getId(), e);
return new TaskResult(TaskResult.Status.FAILED, e.toString());
+ } finally {
+ minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, -1L);
Review comment:
Best to call a method `runInternal` that has the current logic, and manipulate this metric before and after the call outside (in the run() method). That way, if someone adds a return from an earlier point, we don't have to worry about the metric going bad.
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26324d8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **increase** coverage by `0.07%`.
> The diff coverage is `89.47%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
============================================
+ Coverage 73.49% 73.56% +0.07%
Complexity 91 91
============================================
Files 1492 1494 +2
Lines 73433 73587 +154
Branches 10577 10595 +18
============================================
+ Hits 53966 54135 +169
+ Misses 15938 15930 -8
+ Partials 3529 3522 -7
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.75% <89.47%> (+0.16%)` | :arrow_up: |
| unittests | `65.43% <0.00%> (-0.09%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `78.12% <87.50%> (+3.57%)` | :arrow_up: |
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <100.00%> (+78.88%)` | :arrow_up: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `46.98% <0.00%> (-7.23%)` | :arrow_down: |
| [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `69.87% <0.00%> (-6.03%)` | :arrow_down: |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `72.34% <0.00%> (-5.32%)` | :arrow_down: |
| [.../org/apache/pinot/core/startree/StarTreeUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9TdGFyVHJlZVV0aWxzLmphdmE=) | `76.47% <0.00%> (-3.93%)` | :arrow_down: |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `79.41% <0.00%> (-3.55%)` | :arrow_down: |
| [...e/impl/forward/FixedByteMVMutableForwardIndex.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2ZvcndhcmQvRml4ZWRCeXRlTVZNdXRhYmxlRm9yd2FyZEluZGV4LmphdmE=) | `90.90% <0.00%> (-3.04%)` | :arrow_down: |
| [.../controller/helix/core/SegmentDeletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1NlZ21lbnREZWxldGlvbk1hbmFnZXIuamF2YQ==) | `75.80% <0.00%> (-2.07%)` | :arrow_down: |
| ... and [36 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...26324d8](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r661015201
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,59 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ long startTimeMillis = System.currentTimeMillis();
Review comment:
done
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,59 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
Review comment:
done
--
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] [incubator-pinot] mqliang commented on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870991144
@Jackie-Jiang
> Thanks for adding these metrics. After the PR is merged, can you please help update the pinot doc regarding the task metrics?
PR: https://github.com/pinot-contrib/pinot-docs/pull/36 https://github.com/pinot-contrib/pinot-docs/pull/37
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db76bac) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **decrease** coverage by `0.00%`.
> The diff coverage is `90.47%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
============================================
- Coverage 73.49% 73.48% -0.01%
Complexity 91 91
============================================
Files 1492 1495 +3
Lines 73433 73586 +153
Branches 10577 10595 +18
============================================
+ Hits 53966 54074 +108
- Misses 15938 15980 +42
- Partials 3529 3532 +3
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.63% <90.47%> (+0.04%)` | :arrow_up: |
| unittests | `65.42% <0.00%> (-0.09%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `77.41% <88.88%> (+2.87%)` | :arrow_up: |
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <100.00%> (+78.88%)` | :arrow_up: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `69.87% <0.00%> (-6.03%)` | :arrow_down: |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `72.34% <0.00%> (-5.32%)` | :arrow_down: |
| [...operator/filter/RangeIndexBasedFilterOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvUmFuZ2VJbmRleEJhc2VkRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `40.42% <0.00%> (-4.26%)` | :arrow_down: |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `79.41% <0.00%> (-3.55%)` | :arrow_down: |
| [...r/validation/RealtimeSegmentValidationManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci92YWxpZGF0aW9uL1JlYWx0aW1lU2VnbWVudFZhbGlkYXRpb25NYW5hZ2VyLmphdmE=) | `80.64% <0.00%> (-3.23%)` | :arrow_down: |
| [.../controller/helix/core/SegmentDeletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1NlZ21lbnREZWxldGlvbk1hbmFnZXIuamF2YQ==) | `75.80% <0.00%> (-2.07%)` | :arrow_down: |
| [.../helix/core/realtime/SegmentCompletionManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1NlZ21lbnRDb21wbGV0aW9uTWFuYWdlci5qYXZh) | `72.13% <0.00%> (-1.03%)` | :arrow_down: |
| ... and [26 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...db76bac](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660987487
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,19 +71,31 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
Review comment:
helix set the job start time in milliseconds. So if we wanner unify the the granularity of log/gauge, the only option is change the granularity of log as milliseconds.
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
--
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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660789369
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -108,6 +120,8 @@ public TaskResult run() {
minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FAILED, 1L);
LOGGER.error("Caught exception while executing task: {}", _taskConfig.getId(), e);
return new TaskResult(TaskResult.Status.FAILED, e.toString());
+ } finally {
+ minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, -1L);
Review comment:
Done.
--
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] [incubator-pinot] mqliang commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mqliang commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660987487
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,19 +71,31 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
Review comment:
helix set the job start time in milliseconds. So if we wanner unify the the granularity, the only option is change the granularity of TASK_EXECUTION as milliseconds.
--
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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660994187
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,60 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ try {
+ _minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
+ long startTimeMillis = System.currentTimeMillis();
+ TaskResult result = runInternal();
+ long timeSpentInMillis = System.currentTimeMillis() - startTimeMillis;
Review comment:
these two lines should go into finally. even if task fails for whatever reason, it is time spent, and we should count it.
--
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] [incubator-pinot] mcvsubbu merged pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu merged pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099
--
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] [incubator-pinot] jackjlli commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r661013869
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,60 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ try {
+ _minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
+ long startTimeMillis = System.currentTimeMillis();
+ TaskResult result = runInternal();
+ long timeSpentInMillis = System.currentTimeMillis() - startTimeMillis;
Review comment:
nit: rename it to sth like `executionTimeMs`
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,59 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
Review comment:
rename it to `jobInQueueTimeMs`
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,45 +70,59 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ long startTimeMillis = System.currentTimeMillis();
Review comment:
nit: you can reuse the `jobDequeueTime` for calculating `MinionQueryPhase.TASK_EXECUTION`.
--
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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8457967) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **increase** coverage by `0.00%`.
> The diff coverage is `52.42%`.
> :exclamation: Current head 8457967 differs from pull request most recent head 22e9c22. Consider uploading reports for the commit 22e9c22 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
=========================================
Coverage 73.49% 73.49%
Complexity 91 91
=========================================
Files 1492 1493 +1
Lines 73433 73531 +98
Branches 10577 10590 +13
=========================================
+ Hits 53966 54040 +74
- Misses 15938 15955 +17
- Partials 3529 3536 +7
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.61% <52.42%> (+0.02%)` | :arrow_up: |
| unittests | `65.43% <5.82%> (-0.09%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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/controller/ControllerConf.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9Db250cm9sbGVyQ29uZi5qYXZh) | `52.23% <0.00%> (-0.95%)` | :arrow_down: |
| [...ntroller/helix/core/minion/TaskMetricsEmitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9UYXNrTWV0cmljc0VtaXR0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `79.41% <74.00%> (-3.55%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | `97.22% <100.00%> (+0.55%)` | :arrow_up: |
| [...controller/helix/core/minion/PinotTaskManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdFRhc2tNYW5hZ2VyLmphdmE=) | `27.77% <100.00%> (-0.27%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <100.00%> (+78.88%)` | :arrow_up: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <100.00%> (ø)` | |
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `77.41% <100.00%> (+2.87%)` | :arrow_up: |
| [...e/impl/dictionary/LongOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09uSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `69.87% <0.00%> (-6.03%)` | :arrow_down: |
| [.../impl/dictionary/FloatOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPbkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `69.87% <0.00%> (-6.03%)` | :arrow_down: |
| ... and [27 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...22e9c22](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] codecov-commenter edited a comment on pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#issuecomment-870103030
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 [#7099](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26324d8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/0c813bd5edd7d1b714242dcc07076aaf2db8c4d7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0c813bd) will **decrease** coverage by `8.06%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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 #7099 +/- ##
============================================
- Coverage 73.49% 65.43% -8.07%
Complexity 91 91
============================================
Files 1492 1494 +2
Lines 73433 73587 +154
Branches 10577 10595 +18
============================================
- Hits 53966 48148 -5818
- Misses 15938 22036 +6098
+ Partials 3529 3403 -126
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.43% <0.00%> (-0.09%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7099?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/minion/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `0.00% <0.00%> (-11.12%)` | :arrow_down: |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../pinot/minion/taskfactory/TaskFactoryRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vdGFza2ZhY3RvcnkvVGFza0ZhY3RvcnlSZWdpc3RyeS5qYXZh) | `0.00% <0.00%> (-74.55%)` | :arrow_down: |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [353 more](https://codecov.io/gh/apache/incubator-pinot/pull/7099/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/incubator-pinot/pull/7099?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/incubator-pinot/pull/7099?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 [0c813bd...26324d8](https://codecov.io/gh/apache/incubator-pinot/pull/7099?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] [incubator-pinot] mcvsubbu commented on a change in pull request #7099: Add minion metrics of task queueing time and task numbers
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7099:
URL: https://github.com/apache/incubator-pinot/pull/7099#discussion_r660849514
##########
File path: pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java
##########
@@ -67,19 +71,31 @@ public TaskFactoryRegistry(TaskExecutorFactoryRegistry taskExecutorFactoryRegist
private final TaskConfig _taskConfig = context.getTaskConfig();
private final PinotTaskExecutor _taskExecutor = taskExecutorFactory.create();
private final MinionEventObserver _eventObserver = eventObserverFactory.create();
+ private final MinionMetrics _minionMetrics = MinionContext.getInstance().getMinionMetrics();
@Override
public TaskResult run() {
- MinionMetrics minionMetrics = MinionContext.getInstance().getMinionMetrics();
+ HelixManager helixManager = context.getManager();
+ JobContext jobContext = TaskDriver.getJobContext(helixManager, context.getJobConfig().getJobId());
+ // jobContext.getStartTime() return the time in milliseconds of job being put into helix queue.
+ long jobInQueueTime = jobContext.getStartTime();
+ long jobDequeueTime = System.currentTimeMillis();
+ _minionMetrics.addPhaseTiming(taskType, MinionQueryPhase.TASK_QUEUEING, jobDequeueTime - jobInQueueTime);
+ _minionMetrics.addValueToGlobalGauge(MinionGauge.NUMBER_OF_TASKS, 1L);
+ TaskResult result = runInternal();
Review comment:
Can you add a try/finally 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