release-23.1: jobs: avoid crdb_internal.system_jobs in gc-jobs

[blathers-crl]

Backport 1/1 commits from #108093 on behalf of @stevendanna.

/cc @cockroachdb/release

---

The crdb_internal.system_jobs is a virtual table that joins information from the jobs table and the jobs_info table.

For the previous query,

SELECT id, payload, status FROM "".crdb_internal.system_jobs
WHERE (created < $1) AND (id > $2)
ORDER BY id
LIMIT $3

this is a little suboptimal because:

• We don't make use of the progress column so any read of that is useless.

• While the crdb_internal.virtual table has a virtual index on job id, and EXPLAIN will even claim that it will be used:

  • limit
  │ count: 100
  │
  └── • filter
      │ filter: created < '2023-07-20 07:29:01.17001'
      │
      └── • virtual table
            table: system_jobs@system_jobs_id_idx
            spans: [/101 - ]
  

  This is actually a lie. A virtual index can only handle single-key spans. As a result the unconstrained query is used:

    WITH
        latestpayload AS (SELECT job_id, value FROM system.job_info AS payload WHERE info_key = 'legacy_payload' ORDER BY written DESC),
        latestprogress AS (SELECT job_id, value FROM system.job_info AS progress WHERE info_key = 'legacy_progress' ORDER BY written DESC)
    SELECT
       DISTINCT(id), status, created, payload.value AS payload, progress.value AS progress,
                created_by_type, created_by_id, claim_session_id, claim_instance_id, num_runs, last_run, job_type
    FROM system.jobs AS j
    INNER JOIN latestpayload AS payload ON j.id = payload.job_id
    LEFT JOIN latestprogress AS progress ON j.id = progress.job_id

which has a full scan of the jobs table and 2 full scans of the info table:

  • distinct
  │ distinct on: id, value, value
  │
  └── • merge join
      │ equality: (job_id) = (id)
      │
      ├── • render
      │   │
      │   └── • filter
      │       │ estimated row count: 7,318
      │       │ filter: info_key = 'legacy_payload'
      │       │
      │       └── • scan
      │             estimated row count: 14,648 (100% of the table; stats collected 39 minutes ago; using stats forecast for 2 hours in the future)
      │             table: job_info@primary
      │             spans: FULL SCAN
      │
      └── • merge join (right outer)
          │ equality: (job_id) = (id)
          │ right cols are key
          │
          ├── • render
          │   │
          │   └── • filter
          │       │ estimated row count: 7,317
          │       │ filter: info_key = 'legacy_progress'
          │       │
          │       └── • scan
          │             estimated row count: 14,648 (100% of the table; stats collected 39 minutes ago; using stats forecast for 2 hours in the future)
          │             table: job_info@primary
          │             spans: FULL SCAN
          │
          └── • scan
                missing stats
                table: jobs@primary
                spans: FULL SCAN

Because of the limit, I don't think this ends up being as bad as it looks. But it still isn't great.

In this PR, we replace crdb_internal.jobs with a query that removes the join on the unused progress field and also constrains the query of the job_info table.

  • distinct
  │ distinct on: id, value
  │
  └── • merge join
      │ equality: (job_id) = (id)
      │ right cols are key
      │
      ├── • render
      │   │
      │   └── • filter
      │       │ estimated row count: 7,318
      │       │ filter: info_key = 'legacy_payload'
      │       │
      │       └── • scan
      │             estimated row count: 14,646 (100% of the table; stats collected 45 minutes ago; using stats forecast for 2 hours in the future)
      │             table: job_info@primary
      │             spans: [/101/'legacy_payload' - ]
      │
      └── • render
          │
          └── • limit
              │ count: 100
              │
              └── • filter
                  │ filter: created < '2023-07-20 07:29:01.17001'
                  │
                  └── • scan
                        missing stats
                        table: jobs@primary
                        spans: [/101 - ]

In a local example, this does seem faster:

> SELECT id, payload, status, created
> FROM "".crdb_internal.system_jobs
> WHERE (created < '2023-07-20 07:29:01.17001') AND (id > 100) ORDER BY id LIMIT 100;

id | payload | status | created
-----+---------+--------+----------
(0 rows)

Time: 183ms total (execution 183ms / network 0ms)

> WITH
> latestpayload AS (
>     SELECT job_id, value
>     FROM system.job_info AS payload
>     WHERE job_id > 100 AND info_key = 'legacy_payload'
>     ORDER BY written desc
> ),
> jobpage AS (
>     SELECT id, status, created
>     FROM system.jobs
>     WHERE (created < '2023-07-20 07:29:01.17001') and (id > 100)
>     ORDER BY id
>     LIMIT 100
> )
> SELECT distinct (id), latestpayload.value AS payload, status
> FROM jobpage AS j
> INNER JOIN latestpayload ON j.id = latestpayload.job_id;
  id | payload | status
-----+---------+---------
(0 rows)

Time: 43ms total (execution 42ms / network 0ms)

Release note: None

Epic: none

---

Release justification: Bug fix for performance issue that could cause job system contention.

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Patches should only be created for serious issues or test-only changes.
• Patches should not break backwards-compatibility.
• Patches should change as little code as possible.
• Patches should not change on-disk formats or node communication protocols.
• Patches should not add new functionality.
• Patches must not add, edit, or otherwise modify cluster versions; or add version gates.

If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.

• There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
• The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
• New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters.
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.

Add a brief release justification to the body of your PR to justify this backport.

Some other things to consider:

• What did we do to ensure that a user that doesn’t know & care about this backport, has no idea that it happened?
• Will this work in a cluster of mixed patch versions? Did we test that?
• If a user upgrades a patch version, uses this feature, and then downgrades, what happens?

[cockroach-teamcity]

This change is 

[github-actions]

Reminder: it has been 3 weeks please merge or close your backport!

[yuzefovich]

@stevendanna do we want to merge this?

[github-actions]

Reminder: it has been 3 weeks please merge or close your backport!

[stevendanna]

@dt We originally held off on merging this one because it was relatively speculative. I don't think this makes much of a dent but it is increasingly looking like anything we can do for 23.1 here we probably should do, so I've rebased this if you want to take a look.

[stevendanna]

Although, I'm a bit more bullish about #123848 as it might improve the query plan here as a side-effect.

[github-actions]

Reminder: it has been 3 weeks please merge or close your backport!

[yuzefovich]

23.1 branch is done