Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Dashboard: improvements to jobs index and show pages #672

Merged
merged 19 commits into from
Jul 24, 2022

Conversation

bkeepers
Copy link
Contributor

This PR includes some updates I had started working on months ago and finally got around to merging with master. It's mostly a design refresh for the jobs#index list, but includes some updates to jobs#show for consistency. There is still room for improvement here, but I think this is a good step forward.

jobs#index: Beforejobs#index: After
image image
jobs#show: Beforejobs#show: After
image image

app/assets/good_job/modules/popovers.js Show resolved Hide resolved
app/assets/good_job/style.css Show resolved Hide resolved
app/helpers/good_job/application_helper.rb Show resolved Hide resolved
app/views/good_job/jobs/_table.erb Outdated Show resolved Hide resolved
app/views/good_job/jobs/_table.erb Outdated Show resolved Hide resolved
@@ -8,6 +8,7 @@ module GoodJob
class Job < BaseRecord
include Filterable
include Lockable
include Reportable
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe a quibble, but I don't think the latency/runtime stats quite make sense for a Job record because a Job is a container for multiple Executions.

Thinking about my generic recommendations for optimizing queues, I think the number to report is the total latency for the entire job e.g. the time from when the first execution was expected to run, to the time that the job eventually finished.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I'll remove them from the table for now, and…

I'd love to discuss what I was trying to get at by adding them: I'm looking for a snapshot of queue latency and runtime as operational metrics which indicate if there are enough workers/resources. This was just an easy way to quickly scan them to get a rough overview. It makes a lot more sense to include them on an overview dashboard rather than in the jobs list. If I remember right, resque-web had a dashboard that showed current latency for each queue.

Also, I think it makes sense for a Job to sum the queue latency and runtime for each Execution, but I'm not sure total wall clock time from start to finish is a helpful operational metric since it includes intentional backoff. Either way, this total probably only makes sense on individual job pages and an overview dashboard.

@bensheldon bensheldon merged commit 440af06 into bensheldon:main Jul 24, 2022
@bensheldon bensheldon added the enhancement New feature or request label Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants