saltstack / saltstack/salt

[Bug]: pgjsonb purge orphans recent salt_returns rows when minions answer at staggered times

Open
#69,060 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

What happened?

Description

salt/returners/pgjsonb.py:_purge_jobs decides which jids rows to delete with this predicate:

sql = (
    "delete from jids where jid in (select distinct jid from salt_returns"
    " where alter_time < %s)"
)

The subquery selects a jid as soon as any single salt_returns row for that jid is older than the cutoff. For a long-running job whose minions answer at staggered times — a slow orchestrate run, an SSH target with a 2-hour command, a syndic batch — the predicate fires on the first old return and deletes the parent jids row, but the recent salt_returns rows for the same jid stay behind in the source table.

Those rows are now orphans:

  • salt-run jobs.lookup_jid <jid> cannot reconstruct the load (the parent in jids is gone).
  • Anything that JOINs salt_returns to jids sees a broken parent edge.
  • _archive_jobs has the same predicate, so the archive copy ends up holding parent rows whose recent returns were left behind in the source table — an inconsistent backup.

The trigger condition is silent: master keeps running, _purge_jobs returns True, and the operator only finds out when a jobs.lookup_jid request fails or when somebody compares row counts between archive and source.

Setup

  • on-prem machine
  • classic packaging
  • onedir packaging

Any deployment using master_job_cache: pgjsonb with keep_jobs_seconds set, that runs jobs whose returns can land outside a single tick of loop_interval. Long-running orchestrations and SSH-mode runs are typical.

Steps to Reproduce the behavior

  1. Configure master_job_cache: pgjsonb and keep_jobs_seconds: 3600.
  2. Publish a job that takes longer than keep_jobs_seconds to finish on at least one minion. For example: salt --async '*' cmd.run 'sleep 3700'.
  3. Wait for one minion to return early (within seconds) and another to return after keep_jobs_seconds.
  4. The maintenance loop runs _purge_jobs — sees the early return is older than the cutoff, deletes the jids row.
  5. The slow minion's later return INSERTs into salt_returns against a jid whose parent in jids is gone.
  6. salt-run jobs.lookup_jid <jid> returns empty / fails to reconstruct the load.

Expected behavior

A jids row should be deleted only when every salt_returns row for that jid is older than the cutoff. As long as any recent return exists, the parent should be kept.

Additional context

The accompanying PR rewrites both predicates as an EXISTS / NOT EXISTS antijoin so a jids row is removed only when every salt_returns row for that jid is older than the cutoff. EXISTS is preferred over IN (SELECT distinct ...): it short-circuits on the first hit, uses idx_salt_returns_jid for per-row probes, and avoids NOT IN's NULL gotcha. Since jids is typically much smaller than salt_returns, the planner picks an antijoin via the jid index — at least as fast as the original seq scan.

The PR scope is intentionally narrow: it fixes orphan returns only. There is a related but separate slower leak of jids rows that never received any salt_returns at all (a job published against an offline target leaves an immortal jids row). Closing that needs either a jids.created_at schema migration with backwards-compat detection or jid-format parsing in Python; both warrant their own discussion and will be tracked as a follow-up.

Builds on #69049 (which routed pgjsonb errors through Salt's logger and structured the catch blocks the new code re-uses) and should be merged after it.

Type of salt install

Official deb

Major version

3006.x, 3007.x

What supported OS are you seeing the problem on? Can select multiple. (If bug appears on an unsupported OS, please open a GitHub Discussion instead)

debian-11, debian-12

salt --versions-report output
salt --versions-report
Salt Version:
          Salt: 3007.13

Python Version:
        Python: 3.10.19 (main, Feb  5 2026, 07:05:38) [GCC 11.2.0]

Dependency Versions:
          cffi: 2.0.0
      cherrypy: unknown
  cryptography: 42.0.5
      dateutil: 2.8.2
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.6
       libgit2: 1.9.1
  looseversion: 1.3.0
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.7
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 24.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: 1.18.2
  python-gnupg: 0.5.2
        PyYAML: 6.0.1
         PyZMQ: 25.1.2
        relenv: 0.22.3
         smmap: Not Installed
       timelib: 0.3.0
       Tornado: 6.5.4
           ZMQ: 4.3.4

Salt Extensions:
 saltext.vault: 1.5.0

Salt Package Information:
  Package Type: onedir

System Versions:
          dist: debian 12.13 bookworm
        locale: utf-8
       machine: x86_64
       release: 6.12.73+deb12-amd64
        system: Linux
       version: Debian GNU/Linux 12.13 bookworm

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in salt/returners/pgjsonb.py at _purge_jobs and compare its jid deletion predicate with the corresponding predicate in _archive_jobs. Verify that a jid remains while any salt_returns row is recent, then confirm that both purge and archive operations handle staggered returns without orphaning rows or creating an inconsistent backup.

Written by the indexing model from the issue text.

Assessment

Tech stack
postgresql, python, sql
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.