Project

General

Profile

Actions

Bug #3381

closed

[Workbench] "jobs" view in Workbench shows failed job job but green progress bar

Added by Abram Connelly over 9 years ago. Updated over 9 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Story points:
1.0

Description

A job failed and the status in the "jobs" view in Workbench shows it as failed but the proress bar is fully green.

The relevant job is qr1hi-8i9sb-n1yv047kymyjtxs .

Screenshot is attached.

(TC) This is probably caused by Workbench's attempt to show {#done, #failed, #running, #todo} in the progress bar as {green,red,blue,empty} respectively: {150,2,0,0} is likely indistinguishable from {100,0,0,0} in a 100-pixel progress bar. Requirements (which we expect to address this case as well as similar situations like "1 task failed, 999 never attempted"):
  1. Do not show a progress bar at all for a job that is not running right now. Instead, each job should show exactly one status indicator: a progress bar if running, a label if queued/complete/failed. (Currently we display a progress bar and a label for each job, which makes it harder to distinguish states.)
  2. Do not try to express #failed/running in the progress bar. Only show #done. Use green ("success" class in bootstrap) if no tasks have failed yet, orange ("warning" class) if any failures have occurred.
    • Rationale: 0/1000 and 1/1000 are nearly equal if you're measuring doneness, but they're very different if you're measuring failures. Displaying a number of failures as a proportion of total work is not especially sensible.
  3. Near (beside or below) each progress bar, show "A tasks done, B failures, C todo, D running". (This is a better way to provide the additional detail that we previously tried to pack in to the progressbar.)

Incidentally, the dashboard mockups at Workbench UI images show pipelines as sets of tiny "progress or status" blobs. It isn't necessary to match the styling in the mockups -- just offering as a preview / reference point.


Files

failed_success_workbench.png (180 KB) failed_success_workbench.png Abram Connelly, 07/28/2014 10:09 AM
Job-progressbar-incorrect.tiff (190 KB) Job-progressbar-incorrect.tiff Percentage sum is 101% Radhika Chippada, 09/22/2014 02:16 PM

Subtasks 2 (0 open2 closed)

Task #4112: Update pipeline/job page display of job statusResolvedPeter Amstutz09/22/2014Actions
Task #3950: Review branch: 3381-job-progress-bar-bugResolvedPeter Amstutz09/22/2014Actions

Related issues

Related to Arvados - Bug #4085: [Workbench] On pipeline instance page, queued jobs should have Cancel buttonsResolvedActions
Actions #1

Updated by Peter Amstutz over 9 years ago

  • Target version set to Arvados Future Sprints
Actions #2

Updated by Ward Vandewege over 9 years ago

  • Subject changed from "jobs" view in Workbench shows failed job job but green progress bar to [Workbench] "jobs" view in Workbench shows failed job job but green progress bar
Actions #3

Updated by Tom Clegg over 9 years ago

  • Description updated (diff)
  • Category set to Workbench
Actions #4

Updated by Tom Clegg over 9 years ago

  • Story points set to 1.0
Actions #5

Updated by Tom Clegg over 9 years ago

  • Target version changed from Arvados Future Sprints to 2014-10-08 sprint
Actions #6

Updated by Radhika Chippada over 9 years ago

  • Assigned To set to Radhika Chippada
Actions #7

Updated by Radhika Chippada over 9 years ago

  • Status changed from New to In Progress
Actions #8

Updated by Radhika Chippada over 9 years ago

It appears that problem is due to incorrect percentages sum for the progress bar. Please see attached.

When I inspect element for the progress bar for this job, I see that success is computed as 99% and failed as 2% and hence the 2% is not showing up. If I were to manually tweak the failed percentage to 1%, I see red for the failed.

I will look into why the percentage sum is over 100%.

Actions #9

Updated by Radhika Chippada over 9 years ago

Notes on the fix I implemented:

The issue was occurring due to conversion of percentages computed into integers using ceil method. Due to this, in this particular job's case, the total of the percentages computed is > 100% (99% success and 2% failures to be precise, instead of 98.8% success and 1.2% failures) and hence the offending failures percentage was not rendered.

I removed the conversion into integers by removing the ceil method and I now see the failures in red.

Let me know if this is not sufficient, and if we indeed want to display actual numbers instead of the progress bar.

Actions #10

Updated by Tom Clegg over 9 years ago

  • Description updated (diff)
Actions #11

Updated by Brett Smith over 9 years ago

Radhika,

I took a look at 52f0c22, and it was a definite improvement. It would help avoid situations where "failed" progress is nudged off the edge of the bar by rounding. Thanks.

However, I wasn't sure if it would be sufficient to cover some of the related situations that Tom talked about in the description. I pinged him about it, and he clarified which parts he considers requirements for this story. Could you please take a look and update the branch to match? Thank you.

Actions #12

Updated by Peter Amstutz over 9 years ago

  • Assigned To changed from Radhika Chippada to Peter Amstutz
Actions #13

Updated by Peter Amstutz over 9 years ago

See also #3073

Actions #14

Updated by Brett Smith over 9 years ago

Reviewing 2c7b720

I'm getting a bunch of Workbench test failures. There are lots of 422s with a stack trace starting like:

Processing by ProjectsController#index as HTML
  Rendered projects/_show_dashboard.html.erb (178.0ms)
#<TypeError: no implicit conversion of Symbol into Integer>
/home/brett/repos/arvados/apps/workbench/app/views/projects/_show_dashboard.html.erb:36:in `[]'
/home/brett/repos/arvados/apps/workbench/app/views/projects/_show_dashboard.html.erb:36:in `block (2 levels) in _app_views_projects__show_das
hboard_html_erb__2688463059846288523_94558320'
/home/brett/repos/arvados/apps/workbench/app/views/projects/_show_dashboard.html.erb:35:in `each'
/home/brett/repos/arvados/apps/workbench/app/views/projects/_show_dashboard.html.erb:35:in `block in _app_views_projects__show_dashboard_html
_erb__2688463059846288523_94558320'

I don't get these failures on master. Can you please investigate?

Everything else pertains to _running_component.html.erb:

  • If I'm following the blocks correctly, I'm not sure how it's possible to get into the elsif current_job[:state] == "Queued" branch. It looks like a follow-up to if current_job, meaning if we get to the elsif, current_job is either false or nil, and calling [] on it will crash. Am I missing something here? If not, should any of this get rolled into the new Queued state branch?
  • You can use Rails' pluralize method to pluralize "tasks," rather than manually adding an "s".
  • The column offset comments don't seem to be consistent. It seems like they all document what the offset is before the following div, except the first one, which lists 3 instead of 0. Can you please make them consistent, or otherwise clarify what's going on here?

Thanks.

Actions #15

Updated by Peter Amstutz over 9 years ago

Brett Smith wrote:

Reviewing 2c7b720

I'm getting a bunch of Workbench test failures. There are lots of 422s with a stack trace starting like:

[...]

I don't get these failures on master. Can you please investigate?

Fixed

Everything else pertains to _running_component.html.erb:

  • If I'm following the blocks correctly, I'm not sure how it's possible to get into the elsif current_job[:state] == "Queued" branch. It looks like a follow-up to if current_job, meaning if we get to the elsif, current_job is either false or nil, and calling [] on it will crash. Am I missing something here? If not, should any of this get rolled into the new Queued state branch?

You're right, it got screwed up somewhere down the line. I reorganized that whole block of code to fix some problems and it should be easier to follow (and properly indented!)

Fixed.

  • The column offset comments don't seem to be consistent. It seems like they all document what the offset is before the following div, except the first one, which lists 3 instead of 0. Can you please make them consistent, or otherwise clarify what's going on here?

Fixed.

Actions #16

Updated by Brett Smith over 9 years ago

Peter Amstutz wrote:

  • If I'm following the blocks correctly, I'm not sure how it's possible to get into the elsif current_job[:state] "Queued" branch. It looks like a follow-up to if current_job, meaning if we get to the elsif, current_job is either false or nil, and calling [] on it will crash. Am I missing something here? If not, should any of this get rolled into the new Queued state branch?

You're right, it got screwed up somewhere down the line. I reorganized that whole block of code to fix some problems and it should be easier to follow (and properly indented!)

Thanks. The new current_job[:state] "Queued" branch says it's column offset 3, but it's actually 5. Please go ahead and merge with that fixed.

Actions #17

Updated by Anonymous over 9 years ago

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100

Applied in changeset arvados|commit:6d8a27a40903f0dc61876947cecc9401edd3a32c.

Actions

Also available in: Atom PDF