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

Remove unneeded include of pg_locks in query when displaying jobs table #1541

Merged

Conversation

jgrau
Copy link
Contributor

@jgrau jgrau commented Nov 14, 2024

Related to #1540

  • Removes an (unused?) join of the pg_locks table which was slow

@bensheldon regarding your comments

  • The pg_locks could be replaced with a locked_at IS NOT NULL query

I'm not sure but I found that the joined table was not being used for constraints but for including the locktype which seems to be unused(?). I removed it and browsed around the dashboard and found to errors 🤷

  • The COALESCE can be replaced solely with good_jobs.scheduled_at because now all jobs should have a scheduled_at regardless of whether they are expect to run immediately or in the future.

Can you elaborate? There's multiple places where we use coalesce(scheduled_at, created_at) so if that's not needed in general anymore I feel there's a lot of cleanup that could be done and I can do that in this PR with your guidance :)

@bensheldon
Copy link
Owner

Yep, the COALESCENCE is not needed. Prior to GoodJob v4 the scheduled_at column would be null for non-future scheduled jobs. That's no longer the case as of GoodJob v4 (some discussion in #1323 (comment))

I think removing the coalesce might be a slight behavior change in some cases, but I think it might be better to be explicit about whether we intend to order by when the job was first enqueued (created_at) or when the job is intended to be executed even when retried (scheduled_at)

@bensheldon
Copy link
Owner

Maybe let's do the COALESCE removal in a separate PR so I can land this one sooner. I want to poke at it a little, but this looks good 👍

@bensheldon bensheldon added the refactor Code changes that do not introduce new features label Nov 16, 2024
@bensheldon bensheldon changed the title Performance optimisations for dashboard Use column-based status instead of advisory-lock status on dashboard to speed up queries Nov 16, 2024
@bensheldon bensheldon changed the title Use column-based status instead of advisory-lock status on dashboard to speed up queries Do not load job advisory lock status on dashboard to speed up queries Nov 16, 2024
@bensheldon bensheldon changed the title Do not load job advisory lock status on dashboard to speed up queries Do not load job advisory lock status on dashboard: speeds up queries and unused Nov 16, 2024
@bensheldon bensheldon changed the title Do not load job advisory lock status on dashboard: speeds up queries and unused Remove unneeded include of pg_locks in query when displaying jobs table Nov 16, 2024
@bensheldon bensheldon merged commit 74f3bde into bensheldon:main Nov 16, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code changes that do not introduce new features
Projects
Development

Successfully merging this pull request may close these issues.

2 participants