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

feat: New fetch job function. #1241

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

TheNeedForSleep
Copy link

@TheNeedForSleep TheNeedForSleep commented Nov 20, 2024

fixes #1242

New fetch job function:

  • handles on update conflict
  • works with same lock different priority task order
  • inner select uses index properly

Does not support the aborting status!

Successful PR Checklist:

  • Tests
    • (not applicable?)
  • Documentation
    • (not applicable?)

PR label(s):

New fetch job function:
- handles on update conflict
- works with same lock different priority task order
- inner select uses index properly
@TheNeedForSleep TheNeedForSleep requested a review from a team as a code owner November 20, 2024 14:20
@github-actions github-actions bot added the PR type: bugfix 🕵️ Contains bug fix label Nov 20, 2024
Fetch job with retry after max retries get any doable job instead
@TheNeedForSleep
Copy link
Author

The code is not ready to be merged!

@TkTech
Copy link

TkTech commented Dec 3, 2024

The code is not ready to be merged!

You can mark the PR as a draft to make sure it doesn't get accidentally merged :)

@ewjoachim ewjoachim marked this pull request as draft December 4, 2024 13:34
@ewjoachim
Copy link
Member

When the PR is ready, press the big "Ready for review" button

Screenshot 2024-12-04 at 14 34 57

@joaquimds
Copy link

Thanks for this PR, @TheNeedForSleep ! I have just come up against this issue in a project I'm working on. I have temporarily "fixed" it for myself with pretty much the same SQL change:

CREATE OR REPLACE FUNCTION procrastinate_fetch_job(
    target_queue_names character varying[]
)
    RETURNS procrastinate_jobs
    LANGUAGE plpgsql
AS $$
DECLARE
	found_jobs procrastinate_jobs;
BEGIN
    WITH candidate AS (
        SELECT jobs.*
            FROM procrastinate_jobs AS jobs
            WHERE
                (
                    jobs.lock IS NULL OR
                    -- reject the job if its lock has earlier jobs
                    NOT EXISTS (
                        SELECT 1
                            FROM procrastinate_jobs AS earlier_jobs
                            WHERE
                                earlier_jobs.lock = jobs.lock
                                AND earlier_jobs.status IN ('todo', 'doing', 'aborting')
                                AND earlier_jobs.id < jobs.id)
                )
                AND jobs.status = 'todo'
                AND (target_queue_names IS NULL OR jobs.queue_name = ANY( target_queue_names ))
                AND (jobs.scheduled_at IS NULL OR jobs.scheduled_at <= now())
            ORDER BY jobs.priority DESC, jobs.id ASC LIMIT 1
            FOR UPDATE OF jobs SKIP LOCKED
    )
    UPDATE procrastinate_jobs
        SET status = 'doing'
        FROM candidate
        WHERE procrastinate_jobs.id = candidate.id
        RETURNING procrastinate_jobs.* INTO found_jobs;

	RETURN found_jobs;
END;
$$;

Question: why is it necessary to make the change in manager.py? It looks to me that the existing query will already fallback to a job where jobs.lock IS NULL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR type: bugfix 🕵️ Contains bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

procrastinate_fetch_job can end up being slow
5 participants