Show most recent scheduling errors

[satishwaghela]

On the TaskName->TableName page, the SCHEDULING ERRORS was previously TBD. Now we have the API for it: GET /tasks/generator/{tableNameWithType}/{taskType}/debug

[codecov-commenter]

Codecov Report

Merging #9161 (3aa4f98) into master (fb4966b) will decrease coverage by 31.75%.
The diff coverage is 23.76%.

❗ Current head 3aa4f98 differs from pull request most recent head 66d3666. Consider uploading reports for the commit 66d3666 to get more accurate results

@@              Coverage Diff              @@
##             master    #9161       +/-   ##
=============================================
- Coverage     60.15%   28.40%   -31.76%     
+ Complexity     4862       53     -4809     
=============================================
  Files          1836     1845        +9     
  Lines         98217    98791      +574     
  Branches      14928    15040      +112     
=============================================
- Hits          59086    28059    -31027     
- Misses        34704    68035    +33331     
+ Partials       4427     2697     -1730     

Flag	Coverage Δ
integration1	26.26% <22.61%> (?)
integration2	24.80% <22.20%> (-0.05%)	⬇️
unittests1	?

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

Impacted Files	Coverage Δ
.../pinot/broker/api/resources/PinotBrokerLogger.java	0.00% <0.00%> (ø)
...ot/broker/requesthandler/BrokerRequestHandler.java	0.00% <0.00%> (ø)
...mmon/assignment/InstanceAssignmentConfigUtils.java	14.63% <0.00%> (+1.81%)	⬆️
...he/pinot/common/assignment/InstancePartitions.java	40.00% <0.00%> (-1.18%)	⬇️
...ot/common/function/scalar/ArithmeticFunctions.java	16.66% <0.00%> (-52.90%)	⬇️
...e/pinot/common/function/scalar/ArrayFunctions.java	0.00% <0.00%> (-100.00%)	⬇️
...t/common/response/broker/BrokerResponseNative.java	91.12% <ø> (-4.15%)	⬇️
...ava/org/apache/pinot/common/utils/LoggerUtils.java	0.00% <0.00%> (ø)
...ontroller/api/resources/PinotControllerLogger.java	0.00% <0.00%> (ø)
...ources/PinotInstanceAssignmentRestletResource.java	0.00% <0.00%> (ø)
... and 1244 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[npawar]

Just tried this out. I'm not seeing the exception message be picked up. For example, here the response has only errors, and no success TS, but Scheduling Errors remains blank

A sample response will look like this, for your reference:

[
  {
    "tableNameWithType": "githubEvents_OFFLINE",
    "taskType": "MergeRollupTask",
    "mostRecentSuccessRunTS": [],
    "mostRecentErrorRunMessages": {
      "2022-08-05T05:44:00.267Z": "java.lang.IllegalStateException: Dummy exception\n\tat org.apache.pinot.plugin.minion.tasks.mergerollup.MergeRollupTaskGenerator.generateTasks(MergeRollupTaskGenerator.java:135)\n\tat org.apache.pinot.controller.helix.core.minion.PinotTaskManager.scheduleTask(PinotTaskManager.java:536)\n\tat org.apache.pinot.controller.helix.core.minion.PinotTaskManager.scheduleTask(PinotTaskManager.java:620)\n\tat org.apache.pinot.controller.helix.core.minion.CronJobScheduleJob.execute(CronJobScheduleJob.java:53)\n\tat org.quartz.core.JobRunShell.run(JobRunShell.java:202)\n\tat org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573)\n",
      "2022-08-05T05:45:00.005Z": "java.lang.IllegalStateException: Dummy exception\n\tat org.apache.pinot.plugin.minion.tasks.mergerollup.MergeRollupTaskGenerator.generateTasks(MergeRollupTaskGenerator.java:135)\n\tat org.apache.pinot.controller.helix.core.minion.PinotTaskManager.scheduleTask(PinotTaskManager.java:536)\n\tat org.apache.pinot.controller.helix.core.minion.PinotTaskManager.scheduleTask(PinotTaskManager.java:620)\n\tat org.apache.pinot.controller.helix.core.minion.CronJobScheduleJob.execute(CronJobScheduleJob.java:53)\n\tat org.quartz.core.JobRunShell.run(JobRunShell.java:202)\n\tat org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:573)\n"
    }
  }
]

[npawar]

cc @saurabhd336

[npawar]

@joshigaurava please help review

[joshigaurava]

Please eliminate code that is no longer required rather than simply commenting it out.

[npawar]

Looks like the text is spilling out of the Scheduling errors box. Can you just make the text box take up all the space from from left to right, instead of right aligned?
Also, the timezone of the Previous Fire Time and Next Fire Time is incorrect. The response says it is in America/Los_Angeles, but UI shows as GMT

[satishwaghela]

Also, the timezone of the Previous Fire Time and Next Fire Time is incorrect. The response says it is in America/Los_Angeles, but UI shows as GMT

@npawar, Server is sending PreviousFireTime and NextFireTime in epoch format that's why creating date object from that UI shows user's local time.

[npawar]

lgtm functionally

[npawar]

lgtm functionally

Will merge once @joshigaurava approves

[joshigaurava]

My previous comment was to remove unused code. It's just that I didn't point all the places where we need to remove unused code. Please go through the entire PR and remove the code instead of commenting it out.

[joshigaurava]

@satishwaghela can you please post an updated screenshot? For a UI change, it helps in correlating code with the output.

[satishwaghela]

@satishwaghela can you please post an updated screenshot? For a UI change, it helps in correlating code with the output.