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

High database load from job locking query #1549

Open
jonleighton opened this issue Nov 25, 2024 · 14 comments
Open

High database load from job locking query #1549

jonleighton opened this issue Nov 25, 2024 · 14 comments

Comments

@jonleighton
Copy link
Contributor

jonleighton commented Nov 25, 2024

We are seeing high database load from Good Job's locking query.

Here are the four queries, as reported by pghero, with calls / avg runtime / percentage runtime, over a 5 minute period:

Query one (1,881 calls, 45 ms avg, 24%)
SELECT 
  "good_jobs"."id", 
  "good_jobs"."queue_name", 
  "good_jobs"."priority", 
  "good_jobs"."serialized_params", 
  "good_jobs"."scheduled_at", 
  "good_jobs"."performed_at", 
  "good_jobs"."finished_at", 
  "good_jobs"."error", 
  "good_jobs"."created_at", 
  "good_jobs"."updated_at", 
  "good_jobs"."active_job_id", 
  "good_jobs"."concurrency_key", 
  "good_jobs"."cron_key", 
  "good_jobs"."cron_at", 
  "good_jobs"."batch_id", 
  "good_jobs"."batch_callback_id", 
  "good_jobs"."executions_count", 
  "good_jobs"."job_class", 
  "good_jobs"."error_event", 
  "good_jobs"."labels", 
  "good_jobs"."locked_by_id", 
  "good_jobs"."locked_at" 
FROM 
  "good_jobs" 
WHERE 
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT 
        "good_jobs"."id" 
      FROM 
        "good_jobs" 
      WHERE 
        "good_jobs"."queue_name" = $1 
        AND "good_jobs"."finished_at" IS NULL 
        AND (
          "good_jobs"."scheduled_at" <= $2 
          OR "good_jobs"."scheduled_at" IS NULL
        ) 
      ORDER BY 
        priority ASC NULLS LAST, 
        "good_jobs"."created_at" ASC 
      LIMIT 
        $3
    ) 
    SELECT 
      "rows"."id" 
    FROM 
      "rows" 
    WHERE 
      pg_try_advisory_lock(
        (
          $5 || substr(
            md5($6 || $7 || "rows"."id" :: text), 
            $8, 
            $9
          )
        ):: bit(64):: bigint
      ) 
    LIMIT 
      $4
  ) 
ORDER BY 
  priority ASC NULLS LAST, 
  "good_jobs"."created_at" ASC
Query two (1,424 calls, 46 ms avg, 18%)
SELECT 
  "good_jobs"."id", 
  "good_jobs"."queue_name", 
  "good_jobs"."priority", 
  "good_jobs"."serialized_params", 
  "good_jobs"."scheduled_at", 
  "good_jobs"."performed_at", 
  "good_jobs"."finished_at", 
  "good_jobs"."error", 
  "good_jobs"."created_at", 
  "good_jobs"."updated_at", 
  "good_jobs"."active_job_id", 
  "good_jobs"."concurrency_key", 
  "good_jobs"."cron_key", 
  "good_jobs"."cron_at", 
  "good_jobs"."batch_id", 
  "good_jobs"."batch_callback_id", 
  "good_jobs"."executions_count", 
  "good_jobs"."job_class", 
  "good_jobs"."error_event", 
  "good_jobs"."labels", 
  "good_jobs"."locked_by_id", 
  "good_jobs"."locked_at" 
FROM 
  "good_jobs" 
WHERE 
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT 
        "good_jobs"."id" 
      FROM 
        "good_jobs" 
      WHERE 
        "good_jobs"."queue_name" IN ($1, $2) 
        AND "good_jobs"."finished_at" IS NULL 
        AND (
          "good_jobs"."scheduled_at" <= $3 
          OR "good_jobs"."scheduled_at" IS NULL
        ) 
      ORDER BY 
        priority ASC NULLS LAST, 
        "good_jobs"."created_at" ASC 
      LIMIT 
        $4
    ) 
    SELECT 
      "rows"."id" 
    FROM 
      "rows" 
    WHERE 
      pg_try_advisory_lock(
        (
          $6 || substr(
            md5($7 || $8 || "rows"."id" :: text), 
            $9, 
            $10
          )
        ):: bit(64):: bigint
      ) 
    LIMIT 
      $5
  ) 
ORDER BY 
  priority ASC NULLS LAST, 
  "good_jobs"."created_at" ASC
Query three (1,302 calls, 42 ms avg, 15%)
SELECT 
  "good_jobs"."id", 
  "good_jobs"."queue_name", 
  "good_jobs"."priority", 
  "good_jobs"."serialized_params", 
  "good_jobs"."scheduled_at", 
  "good_jobs"."performed_at", 
  "good_jobs"."finished_at", 
  "good_jobs"."error", 
  "good_jobs"."created_at", 
  "good_jobs"."updated_at", 
  "good_jobs"."active_job_id", 
  "good_jobs"."concurrency_key", 
  "good_jobs"."cron_key", 
  "good_jobs"."cron_at", 
  "good_jobs"."batch_id", 
  "good_jobs"."batch_callback_id", 
  "good_jobs"."executions_count", 
  "good_jobs"."job_class", 
  "good_jobs"."error_event", 
  "good_jobs"."labels", 
  "good_jobs"."locked_by_id", 
  "good_jobs"."locked_at" 
FROM 
  "good_jobs" 
WHERE 
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT 
        "good_jobs"."id" 
      FROM 
        "good_jobs" 
      WHERE 
        "good_jobs"."queue_name" IN ($1, $2, $3) 
        AND "good_jobs"."finished_at" IS NULL 
        AND (
          "good_jobs"."scheduled_at" <= $4 
          OR "good_jobs"."scheduled_at" IS NULL
        ) 
      ORDER BY 
        priority ASC NULLS LAST, 
        "good_jobs"."created_at" ASC 
      LIMIT 
        $5
    ) 
    SELECT 
      "rows"."id" 
    FROM 
      "rows" 
    WHERE 
      pg_try_advisory_lock(
        (
          $7 || substr(
            md5($8 || $9 || "rows"."id" :: text), 
            $10, 
            $11
          )
        ):: bit(64):: bigint
      ) 
    LIMIT 
      $6
  ) 
ORDER BY 
  priority ASC NULLS LAST, 
  "good_jobs"."created_at" ASC
Query four (1,050 calls, 42 ms avg, 12%)
SELECT 
  "good_jobs"."id", 
  "good_jobs"."queue_name", 
  "good_jobs"."priority", 
  "good_jobs"."serialized_params", 
  "good_jobs"."scheduled_at", 
  "good_jobs"."performed_at", 
  "good_jobs"."finished_at", 
  "good_jobs"."error", 
  "good_jobs"."created_at", 
  "good_jobs"."updated_at", 
  "good_jobs"."active_job_id", 
  "good_jobs"."concurrency_key", 
  "good_jobs"."cron_key", 
  "good_jobs"."cron_at", 
  "good_jobs"."batch_id", 
  "good_jobs"."batch_callback_id", 
  "good_jobs"."executions_count", 
  "good_jobs"."job_class", 
  "good_jobs"."error_event", 
  "good_jobs"."labels", 
  "good_jobs"."locked_by_id", 
  "good_jobs"."locked_at" 
FROM 
  "good_jobs" 
WHERE 
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT 
        "good_jobs"."id" 
      FROM 
        "good_jobs" 
      WHERE 
        "good_jobs"."finished_at" IS NULL 
        AND (
          "good_jobs"."scheduled_at" <= $1 
          OR "good_jobs"."scheduled_at" IS NULL
        ) 
      ORDER BY 
        priority ASC NULLS LAST, 
        "good_jobs"."created_at" ASC 
      LIMIT 
        $2
    ) 
    SELECT 
      "rows"."id" 
    FROM 
      "rows" 
    WHERE 
      pg_try_advisory_lock(
        (
          $4 || substr(
            md5($5 || $6 || "rows"."id" :: text), 
            $7, 
            $8
          )
        ):: bit(64):: bigint
      ) 
    LIMIT 
      $3
  ) 
ORDER BY 
  priority ASC NULLS LAST, 
  "good_jobs"."created_at" ASC

The only difference between these queries is the "good_jobs"."queue_name" constraints.

Our queues are configured like so:

latency_10s:2; latency_5m,latency_10s:1; latency_1h,latency_5m,latency_10s:1; *:1

The four queries clearly correspond to each of those worker configurations.

We currently run four worker processes (so 4*5 = 20 total threads).

During a 5 minute period, we processed approximately 1,000 jobs. The sum of the query calls is 5,657.

Without being super familiar with Good Job's implementation details, my understanding is that this query obtains a lock for the job it is about to perform. Obtaining the lock may fail, in which case a worker will need to try again.

Perhaps each worker is racing to obtain a lock for a new job as it gets pushed onto the queue? But only one worker can actually obtain that lock.

Is this expected? I wonder if there is a way to structure this so that it doesn't require quite so many queries?

In the mean time, we are going to try reducing our job retention time. This presumably won't reduce the number of queries, but perhaps it will make them run faster and take up less cumulative time. We currently retain jobs for 5 days, and we have ~1.7m jobs preserved over that period.

Note that we already have GOOD_JOB_QUEUE_SELECT_LIMIT=1000 set.

We are using Postgres 14.

@jonleighton
Copy link
Contributor Author

We upgraded to Postgres 16, and these queries are now running a bit faster (more like ~35ms avg). However, this may also have been due to us reducing job retention to 1 day.

@bensheldon
Copy link
Owner

bensheldon commented Dec 2, 2024

This is unfortunately known. The GoodJob locking query has to materialize the inner CTE query, which can be expensive. You could try reducing the value of GOOD_JOB_QUEUE_SELECT_LIMIT=100 (this only has to be as large as your total job concurrency), which would reduce the size of the materialized CTE.

Another maintenance thing you might try is running REINDEX TABLE CONCURRENTLY good_jobs; which I've heard could speed things up (let me know cause I could turn that into a supported thing GoodJob does itself.

Also, I'm looking at the indexes and I'm wondering if there could be a more efficient index (the query has changed a little bit in GoodJob v4):

good_job/demo/db/schema.rb

Lines 100 to 103 in 81d824a

t.index ["priority", "created_at"], name: "index_good_job_jobs_for_candidate_lookup", where: "(finished_at IS NULL)"
t.index ["priority", "created_at"], name: "index_good_jobs_jobs_on_priority_created_at_when_unfinished", order: { priority: "DESC NULLS LAST" }, where: "(finished_at IS NULL)"
t.index ["priority", "scheduled_at"], name: "index_good_jobs_on_priority_scheduled_at_unfinished_unlocked", where: "((finished_at IS NULL) AND (locked_by_id IS NULL))"
t.index ["queue_name", "scheduled_at"], name: "index_good_jobs_on_queue_name_and_scheduled_at", where: "(finished_at IS NULL)"

Cause I'm not seeing a good index specifically for the queue_name-limited situation.

(also, it's unfortunate that Covering Indexes only are supported in Rails 7.2+)

@bensheldon
Copy link
Owner

bensheldon commented Dec 2, 2024

fyi about:

Without being super familiar with Good Job's implementation details, my understanding is that this query obtains a lock for the job it is about to perform. Obtaining the lock may fail, in which case a worker will need to try again.

The query will atomically fetch-and-lock the next runnable job that exists in the database.

  • The inner CTE does query all runnable jobs (WITH "rows"...)
  • The outer query then tries to take a lock on each job returned by the inner query (SELECT ... WHERE pg_try_advisory_lock...)...
  • And returns the first job it is successfully able to acquire a lock on.

So while it does do what you described, it does that entirely within a single database query using a materialized CTE.

@bjeanes
Copy link

bjeanes commented Dec 2, 2024

(also, it's unfortunate that Covering Indexes only are supported in Rails 7.2+)

That only affects the DSL to create them in migrations though. Since GoodJob is Postgres specific, there is no reason it couldn't create those indices directly, right? Some care may need to be taken with-regard-to how execute could interact with strong_migrations in apps, but in theory this isn't a blocker for using those kinds of indices...

@bensheldon
Copy link
Owner

That only affects the DSL to create them in migrations though.

Not a true blocker. Though one of my constraints is not forcing structure.sql. So it it doesn't fit into the Rails schema.rb DSL, I don't want to do it.

@bjeanes
Copy link

bjeanes commented Dec 3, 2024

one of my constraints is not forcing structure.sql

Ah that's fair enough. I personally never use schema.rb because of this lossy factor (and that I really doubt people switch from one DB to another), but I respect the need to maintain compatibility with it here. 👍🏻

It may still warrant testing if it improves query performance and making it a suggestion for people to add themselves.

@jonleighton
Copy link
Contributor Author

Thanks for the replies @bensheldon.

Cause I'm not seeing a good index specifically for the queue_name-limited situation.

I tried to investigate this a bit.

I generated a query without so many variables like so:

GoodJob::Job.queue_string("latency_10s").unfinished.dequeueing_ordered(GoodJob::Job.queue_parser("latency_10s")).only_scheduled.limit(1).advisory_lock

This was based on reading how the code generates the query. It actually runs a query rather than printing it out, but I was able to use the logging output to grab it.

I then formatted it and replaced a few variables to get this
SELECT 
  "good_jobs"."id", 
  "good_jobs"."queue_name", 
  "good_jobs"."priority", 
  "good_jobs"."serialized_params", 
  "good_jobs"."scheduled_at", 
  "good_jobs"."performed_at", 
  "good_jobs"."finished_at", 
  "good_jobs"."error", 
  "good_jobs"."created_at", 
  "good_jobs"."updated_at", 
  "good_jobs"."active_job_id", 
  "good_jobs"."concurrency_key", 
  "good_jobs"."cron_key", 
  "good_jobs"."cron_at", 
  "good_jobs"."batch_id", 
  "good_jobs"."batch_callback_id", 
  "good_jobs"."executions_count", 
  "good_jobs"."job_class", 
  "good_jobs"."error_event", 
  "good_jobs"."labels", 
  "good_jobs"."locked_by_id", 
  "good_jobs"."locked_at" 
FROM 
  "good_jobs" 
WHERE 
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT 
        "good_jobs"."id" 
      FROM 
        "good_jobs" 
      WHERE 
        "good_jobs"."queue_name" = 'latency_10s' 
        AND "good_jobs"."finished_at" IS NULL 
        AND (
          "good_jobs"."scheduled_at" <= now() 
          OR "good_jobs"."scheduled_at" IS NULL
        ) 
      ORDER BY 
        priority ASC NULLS LAST, 
        "good_jobs"."created_at" ASC
    ) 
    SELECT 
      "rows"."id" 
    FROM 
      "rows" 
    WHERE 
      pg_try_advisory_lock(
        (
          'x' || substr(
            md5(
              'good_jobs' || '-' || "rows"."id" :: text
            ), 
            1, 
            16
          )
        ):: bit(64):: bigint
      ) 
    LIMIT 
      1
  ) 
ORDER BY 
  priority ASC NULLS LAST, 
  "good_jobs"."created_at" ASC
I then did `EXPLAIN (ANALYZE, COSTS, VERBOSE, BUFFERS)`
QUERY PLAN
Sort  (cost=16568.96..16568.96 rows=1 width=850) (actual time=10.293..10.296 rows=1 loops=1)
"  Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at"
"  Sort Key: good_jobs.priority, good_jobs.created_at"
  Sort Method: quicksort  Memory: 25kB
  Buffers: shared hit=7828 read=1
  ->  Nested Loop  (cost=16566.92..16568.95 rows=1 width=850) (actual time=10.278..10.282 rows=1 loops=1)
"        Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at"
        Inner Unique: true
        Buffers: shared hit=7828 read=1
        ->  HashAggregate  (cost=16566.49..16566.50 rows=1 width=16) (actual time=9.939..9.942 rows=1 loops=1)
              Output: rows.id
              Group Key: rows.id
              Batches: 1  Memory Usage: 24kB
              Buffers: shared hit=7825
              ->  Limit  (cost=16566.35..16566.49 rows=1 width=16) (actual time=9.927..9.930 rows=1 loops=1)
                    Output: rows.id
                    Buffers: shared hit=7825
                    CTE rows
                      ->  Sort  (cost=16532.96..16566.35 rows=13355 width=28) (actual time=9.828..9.829 rows=1 loops=1)
"                            Output: good_jobs_1.id, good_jobs_1.priority, good_jobs_1.created_at"
"                            Sort Key: good_jobs_1.priority, good_jobs_1.created_at"
                            Sort Method: quicksort  Memory: 25kB
                            Buffers: shared hit=7825
                            ->  Index Scan using index_good_jobs_on_queue_name_and_scheduled_at on public.good_jobs good_jobs_1  (cost=0.41..15617.80 rows=13355 width=28) (actual time=2.562..9.813 rows=2 loops=1)
"                                  Output: good_jobs_1.id, good_jobs_1.priority, good_jobs_1.created_at"
                                  Index Cond: (good_jobs_1.queue_name = 'latency_10s'::text)
                                  Filter: ((good_jobs_1.scheduled_at <= now()) OR (good_jobs_1.scheduled_at IS NULL))
                                  Rows Removed by Filter: 4142
                                  Buffers: shared hit=7825
                    ->  CTE Scan on rows  (cost=0.00..634.36 rows=4452 width=16) (actual time=9.926..9.926 rows=1 loops=1)
                          Output: rows.id
"                          Filter: pg_try_advisory_lock(((('x'::text || substr(md5(('good_jobs-'::text || (rows.id)::text)), 1, 16)))::bit(64))::bigint)"
                          Buffers: shared hit=7825
        ->  Index Scan using good_jobs_pkey on public.good_jobs  (cost=0.42..2.44 rows=1 width=850) (actual time=0.333..0.333 rows=1 loops=1)
"              Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.retried_good_job_id, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.is_discrete, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at"
              Index Cond: (good_jobs.id = rows.id)
              Buffers: shared hit=3 read=1
Query Identifier: 7546228750055156248
Planning:
  Buffers: shared hit=4
Planning Time: 0.564 ms
Execution Time: 10.395 ms

Note that this is run against a replica of our production database, so it might not reflect the actual numbers in production (but the data is the same).

Here's a visualisation.

The major time spent is in the index scan of index_good_jobs_on_queue_name_and_scheduled_at.

Cause I'm not seeing a good index specifically for the queue_name-limited situation.

I don't think adding another index would help, as index_good_jobs_on_queue_name_and_scheduled_at is already used?

You could try reducing the value of GOOD_JOB_QUEUE_SELECT_LIMIT=100 (this only has to be as large as your total job concurrency), which would reduce the size of the materialized CTE.

We could try this, but we don't generally have many jobs sitting in the queue, so this query will already not return many rows. This seems like more of a worst-case optimisation for when the queues get backed up?

Another maintenance thing you might try is running REINDEX TABLE CONCURRENTLY good_jobs; which I've heard could speed things up (let me know cause I could turn that into a supported thing GoodJob does itself.

Happy to try it.

The other thing we do plan to do is change our job retention to be very low, now that #1550 is merged - we'll retain discarded jobs a bit longer but succeeded jobs can be pretty ephemeral. This should make the overall size of our good_jobs table pretty small, which I imagine will speed up that query. I can let you know how this goes - I wonder if it could be a better default behaviour than the present default of storing all jobs for 14 days. I don't think it's that useful to store succeeded jobs, and it has performance impacts both for this query and the admin dashboard.

@jonleighton
Copy link
Contributor Author

fyi about:

Without being super familiar with Good Job's implementation details, my understanding is that this query obtains a lock for the job it is about to perform. Obtaining the lock may fail, in which case a worker will need to try again.

The query will atomically fetch-and-lock the next runnable job that exists in the database.

* The inner CTE does query _all_ runnable jobs (`WITH "rows"...`)

* The outer query then tries to take a lock on each job returned by the inner query (`SELECT ... WHERE pg_try_advisory_lock`...)...

* And returns the first job it is successfully able to acquire a lock on.

So while it does do what you described, it does that entirely within a single database query using a materialized CTE.

Thanks for this explanation. So it is essentially looping over all runnable jobs to find the first one that can be grabbed.

What happens if it doesn't get any jobs? (I.e. the queue is empty, all jobs are locked.)

Does the work immediately try to re-run the query? Should it wait a bit or something?

I wonder if this explains why the query gets run so many times?

@bjeanes
Copy link

bjeanes commented Dec 3, 2024

Note that this is run against a replica of our production database, so it might not reflect the actual numbers in production (but the data is the same).

I ran it against primary, because IME the replica's do have very different statistics:

=*> EXPLAIN (ANALYSE, COSTS, VERBOSE, BUFFERS) SELECT
  "good_jobs"."id",
  "good_jobs"."queue_name",
  "good_jobs"."priority",
  "good_jobs"."serialized_params",
  "good_jobs"."scheduled_at",
  "good_jobs"."performed_at",
  "good_jobs"."finished_at",
  "good_jobs"."error",
  "good_jobs"."created_at",
  "good_jobs"."updated_at",
  "good_jobs"."active_job_id",
  "good_jobs"."concurrency_key",
  "good_jobs"."cron_key",
  "good_jobs"."cron_at",
  "good_jobs"."batch_id",
  "good_jobs"."batch_callback_id",
  "good_jobs"."executions_count",
  "good_jobs"."job_class",
  "good_jobs"."error_event",
  "good_jobs"."labels",
  "good_jobs"."locked_by_id",
  "good_jobs"."locked_at"
FROM
  "good_jobs"
WHERE
  "good_jobs"."id" IN (
    WITH "rows" AS MATERIALIZED (
      SELECT
        "good_jobs"."id"
      FROM
        "good_jobs"
      WHERE
        "good_jobs"."queue_name" = 'latency_10s'
        AND "good_jobs"."finished_at" IS NULL
        AND (
          "good_jobs"."scheduled_at" <= now()
          OR "good_jobs"."scheduled_at" IS NULL
        )
      ORDER BY
        priority ASC NULLS LAST,
        "good_jobs"."created_at" ASC
    )
    SELECT
      "rows"."id"
    FROM
      "rows"
    WHERE
      pg_try_advisory_lock(
        (
          'x' || substr(
            md5(
              'good_jobs' || '-' || "rows"."id" :: text
            ),
            1,
            16
          )
        ):: bit(64):: bigint
      )
    LIMIT
      1
  )
ORDER BY
  priority ASC NULLS LAST,
  "good_jobs"."created_at" ASC
;
                                                                                                                                                                                                                                                                                       QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Sort  (cost=7905.86..7905.87 rows=1 width=851) (actual time=0.637..0.639 rows=0 loops=1)
   Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at
   Sort Key: good_jobs.priority, good_jobs.created_at
   Sort Method: quicksort  Memory: 25kB
   Buffers: shared hit=409
   ->  Nested Loop  (cost=7903.82..7905.85 rows=1 width=851) (actual time=0.633..0.635 rows=0 loops=1)
         Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at
         Inner Unique: true
         Buffers: shared hit=409
         ->  HashAggregate  (cost=7903.40..7903.41 rows=1 width=16) (actual time=0.633..0.634 rows=0 loops=1)
               Output: rows.id
               Group Key: rows.id
               Batches: 1  Memory Usage: 24kB
               Buffers: shared hit=409
               ->  Limit  (cost=7903.26..7903.40 rows=1 width=16) (actual time=0.631..0.633 rows=0 loops=1)
                     Output: rows.id
                     Buffers: shared hit=409
                     CTE rows
                       ->  Sort  (cost=7885.02..7903.26 rows=7296 width=28) (actual time=0.630..0.631 rows=0 loops=1)
                             Output: good_jobs_1.id, good_jobs_1.priority, good_jobs_1.created_at
                             Sort Key: good_jobs_1.priority, good_jobs_1.created_at
                             Sort Method: quicksort  Memory: 25kB
                             Buffers: shared hit=409
                             ->  Bitmap Heap Scan on public.good_jobs good_jobs_1  (cost=114.19..7416.87 rows=7296 width=28) (actual time=0.626..0.627 rows=0 loops=1)
                                   Output: good_jobs_1.id, good_jobs_1.priority, good_jobs_1.created_at
                                   Recheck Cond: (((good_jobs_1.queue_name = 'latency_10s'::text) AND (good_jobs_1.scheduled_at <= now()) AND (good_jobs_1.finished_at IS NULL)) OR ((good_jobs_1.scheduled_at IS NULL) AND (good_jobs_1.finished_at IS NULL)))
                                   Filter: ((good_jobs_1.queue_name = 'latency_10s'::text) AND ((good_jobs_1.scheduled_at <= now()) OR (good_jobs_1.scheduled_at IS NULL)))
                                   Heap Blocks: exact=403
                                   Buffers: shared hit=409
                                   ->  BitmapOr  (cost=114.19..114.19 rows=7296 width=0) (actual time=0.096..0.097 rows=0 loops=1)
                                         Buffers: shared hit=6
                                         ->  Bitmap Index Scan on index_good_jobs_on_queue_name_and_scheduled_at  (cost=0.00..109.25 rows=7296 width=0) (actual time=0.091..0.091 rows=484 loops=1)
                                               Index Cond: ((good_jobs_1.queue_name = 'latency_10s'::text) AND (good_jobs_1.scheduled_at <= now()))
                                               Buffers: shared hit=4
                                         ->  Bitmap Index Scan on index_good_jobs_on_scheduled_at  (cost=0.00..1.30 rows=1 width=0) (actual time=0.004..0.004 rows=0 loops=1)
                                               Index Cond: (good_jobs_1.scheduled_at IS NULL)
                                               Buffers: shared hit=2
                     ->  CTE Scan on rows  (cost=0.00..346.56 rows=2432 width=16) (actual time=0.631..0.631 rows=0 loops=1)
                           Output: rows.id
                           Filter: pg_try_advisory_lock(((('x'::text || substr(md5(('good_jobs-'::text || (rows.id)::text)), 1, 16)))::bit(64))::bigint)
                           Buffers: shared hit=409
         ->  Index Scan using good_jobs_pkey on public.good_jobs  (cost=0.42..2.44 rows=1 width=851) (never executed)
               Output: good_jobs.id, good_jobs.queue_name, good_jobs.priority, good_jobs.serialized_params, good_jobs.scheduled_at, good_jobs.performed_at, good_jobs.finished_at, good_jobs.error, good_jobs.created_at, good_jobs.updated_at, good_jobs.active_job_id, good_jobs.concurrency_key, good_jobs.cron_key, good_jobs.retried_good_job_id, good_jobs.cron_at, good_jobs.batch_id, good_jobs.batch_callback_id, good_jobs.is_discrete, good_jobs.executions_count, good_jobs.job_class, good_jobs.error_event, good_jobs.labels, good_jobs.locked_by_id, good_jobs.locked_at
               Index Cond: (good_jobs.id = rows.id)
 Query Identifier: 7546228750055156248
 Planning:
   Buffers: shared hit=4
 Planning Time: 0.457 ms
 Execution Time: 0.718 ms
(49 rows)

Here is a visualisation based on a JSON version of the above: https://explain.dalibo.com/plan/12b14990491eb039

@jonleighton
Copy link
Contributor Author

Another maintenance thing you might try is running REINDEX TABLE CONCURRENTLY good_jobs; which I've heard could speed things up (let me know cause I could turn that into a supported thing GoodJob does itself.

Happy to try it.

We tried this and pghero is now showing average query time around the ~30ms mark. Which I'm slightly confused about because both those query plans show ~10ms. (Note also that the second plan was done after the reindex.)

@jonleighton
Copy link
Contributor Author

The other thing we do plan to do is change our job retention to be very low, now that #1550 is merged - we'll retain discarded jobs a bit longer but succeeded jobs can be pretty ephemeral. This should make the overall size of our good_jobs table pretty small, which I imagine will speed up that query. I can let you know how this goes - I wonder if it could be a better default behaviour than the present default of storing all jobs for 14 days. I don't think it's that useful to store succeeded jobs, and it has performance impacts both for this query and the admin dashboard.

Just to report back about this, we have turned off GJ's job cleanup stuff, and implemented our own maintenance job that basically does:

GoodJob.cleanup_preserved_jobs(older_than: 1.minute, include_discarded: false)
GoodJob.cleanup_preserved_jobs(older_than: 5.days, include_discarded: true)

Ahead of the deploy, we manually deleted job records older than 1 minute.

We have now seen that query drop to around 2-3ms average runtime, which is a significant improvement.

Unfortunately the job dashboard is still pretty slow to load, even though we now have only ~17k job records. (Most of them are future-scheduled jobs - we could probably implement a better design for the functionality that needs this in our app, but at the moment we are reliant on it and that's what keeps the table a bit bigger.)

I think it would be worth considering making this sort of thing the default behaviour for retention in Good Job. Keeping fewer succeeded jobs around makes many problems go away. (Although I guess functionality like the 'Performance' tab in the dashboard would need a different way of storing the data - but that could be rolled up metrics in a separate table.)

@bensheldon
Copy link
Owner

That's strange that the query is so slow. This is unfortunately one of those things about relational databases: there is a lot more knobs to twist for tuning.

I looked at the performance of this query on the GoodJob Demo, which usually has ~100k job rows and does about 30k of these queries an hour... and is on a fairly underpowered Heroku shared database (essential-0). It's averaging ~1ms on this query:

https://goodjob-demo.herokuapp.com/pghero/queries/6923404554479764969?user=udne4cvup0tecn

...though that query is not using named queues.

Your sharing the EXPLAIN ANALYZE is really helpful. I think the issue here is more that we don't have the right index when queue_name is used in the query. The Bitmap Index Scan / BitmapOr it's doing doesn't look very efficient (and a heap scan!)

Would you be willing to try adding a new index?

add_index :good_jobs, ["queue_name", "priority", "created_at"], name: "index_good_job_jobs_for_candidate_lookup_by_queue", where: "(finished_at IS NULL)"

@bjeanes
Copy link

bjeanes commented Dec 5, 2024

Hey @bensheldon, we'll add that index and report back. Since we lowered our retention the numbers already look much better, but we can at least get a before/after query plan and I'd be open to re-raising our retention temporarily to see how the index fairs.

@jonleighton
Copy link
Contributor Author

jonleighton commented Dec 16, 2024

@bensheldon

We added the index you suggested, here are before/after plans:

Before:

After (we also did VACUUM ANALYSE):

It doesn't look like the new index is used in the plan, but bear in mind that our jobs table currently has only ~18k rows.

If you think it would be useful, we could try increasing our job retention again so that the table gets bigger, and then see if the index is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Inbox
Development

No branches or pull requests

3 participants