What I found auditing 79 background jobs

I audited every background job in a multi-tenant Rails application — 79 job classes, a 49-entry cron schedule, and a Postgres-backed queue — expecting to find a scaling problem. What I actually found was a pile of small, boring correctness bugs, several of which had been quietly costing real work for months.

None of them were exotic. All of them are the kind of thing you write by accident when a queue library's API reads more confidently than it behaves.

This is the long version: seven lessons, the code for each, and — more useful than any of them — the verification habit that repeatedly proved me wrong while I was writing the fixes.

Everything here is generalised. The specifics of the application don't matter; the failure modes are the point, and they show up in any job system with a similar shape.

The setup

The relevant details, and only these:

• Rails 8, jobs on GoodJob — an ActiveJob adapter backed by Postgres rather than Redis.
• Jobs split across a handful of named queues, each with a fixed thread count.
• A cron schedule, mostly per-minute and per-15-minute sweeps.
• Multi-tenant: one database, a tenant_id on everything.

The Postgres-backed part matters for two of the lessons. The rest apply to Sidekiq, Que, Oban, Celery, or anything else where work happens off the request thread.

Lesson 1: your concurrency guard probably permits the thing it names

GoodJob offers three ways to say "only one of these at a time":

good_job_control_concurrency_with(
  enqueue_limit: 1,   # ...one what?
  perform_limit: 1,
  total_limit:   1,
  key: -> { "nightly-rollup" },
)

Sixteen cron jobs in this codebase used enqueue_limit: 1 and were described in comments as singletons. They were not.

Read the gem's own query. enqueue_limit counts jobs matching the key that are unfinished and not currently locked by a worker:

# roughly what the enqueue-stage check counts
GoodJob::Job.where(concurrency_key: key)
            .advisory_unlocked
            .where(locked_by_id: nil)

A job that is executing right now holds an advisory lock, so it is excluded from that count. The guard means "don't let two copies sit in the queue" — not "don't let two copies run".

For a per-minute cron sweep, that is exactly backwards. The tick you want to drop is the one that arrives while the previous tick is still running.

It gets worse in the other direction. perform_limit does check at execution time, but a breach raises ConcurrencyExceededError, and the gem registers:

retry_on GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError,
         attempts: Float::INFINITY

So a cron singleton guarded with perform_limit: 1 doesn't drop the redundant tick — it spins, re-enqueueing forever behind a job that may run for minutes.

The one that behaves like a singleton is total_limit, which resolves to both stages:

class NightlyRollupJob < ApplicationJob
  include GoodJob::ActiveJobExtensions::Concurrency

  # total_limit, NOT enqueue_limit: enqueue_limit excludes a job that is
  # already performing, which is precisely the overlap a cron guard exists
  # to prevent. And NOT perform_limit: a perform-stage breach is retried
  # with attempts: Float::INFINITY, so the redundant tick spins instead of
  # being dropped.
  good_job_control_concurrency_with(
    total_limit: 1,
    key: -> { "nightly-rollup" },
  )
end

The generalisable lesson: when a library gives you three similar knobs, read the query it generates and the error path it takes on breach. The names describe intent; only the code describes behaviour. I got this wrong in my own first draft of the fix and only caught it by opening the gem.

I also added a test that walks the cron schedule and asserts every entry declares a guard, because a rule enforced by comments decays at the first new cron job:

it "every cron job declares a concurrency guard" do
  offenders = cron_job_classes.reject do |klass|
    klass.good_job_concurrency_config[:total_limit].present?
  end

  expect(offenders).to be_empty,
    "These cron jobs can run concurrently with themselves: #{offenders.join(', ')}"
end

Lesson 2: never hold a transaction open across a third-party call

This was the most expensive pattern in the codebase, and it looks completely reasonable:

# DON'T
Message.transaction do
  message = Message.lock.find(id)          # row lock taken
  return if message.sent?                  # idempotence check
  response = MailProvider.deliver(message) # ...an HTTP call. Inside the lock.
  message.update!(sent_at: Time.current, provider_id: response.id)
end

The intent is honest: don't mark it sent unless it sent, and don't let two workers send the same message. But the transaction is now open for the entire duration of a network call to someone else's server.

Three consequences, in ascending order of nastiness:

1. A pooled database connection is pinned for the length of an SMTP conversation. At a 60-second timeout and a few hundred rows, that's your connection pool.
2. Row locks are held for that same duration, so anything touching those rows queues behind a third party's latency.
3. VACUUM stops reclaiming dead tuples across the entire database. An open transaction holds back the xmin horizon. One slow send doesn't bloat the messages table — it bloats every table, because Postgres can't prove no one still needs those old row versions.

The third one is the one people miss, and it's the one that turns "our email is a bit slow" into "why is the database twice the size it was last month".

The fix is to separate claiming the work from doing it. Claim in one committed statement, then send outside any transaction:

CLAIM_LEASE = 10.minutes

def claim(id)
  now = Time.current

  # One conditional UPDATE. The check and the write cannot interleave, so
  # exactly one worker moves the row from "unclaimed or expired" to "claimed"
  # and everyone else gets 0 rows back and skips. No transaction stays open:
  # update_all commits on its own.
  claimed = Message.where(id: id, status: "queued")
                   .where("claimed_until IS NULL OR claimed_until < ?", now)
                   .update_all(claimed_until: now + CLAIM_LEASE, updated_at: now)

  return nil unless claimed == 1

  Message.find_by(id: id)
end

def perform(id)
  message = claim(id) or return   # someone else has it, or it isn't ready

  response = MailProvider.deliver(message)   # no transaction open here
  message.update!(sent_at: Time.current, provider_id: response.id)
end

Why a lease (claimed_until) rather than a status column or a row lock:

• A row lock dies with the connection and requires the transaction to stay open — the thing we're trying to avoid.
• A plain status = 'sending' flag never recovers: a worker that dies mid-send leaves the row stuck forever, and now you need a second job to un-stick it.
• A lease expires on its own. A worker that dies strands nothing; the row becomes claimable again once the lease lapses.

Two details that cost me real bugs:

Release the lease when you defer. If a transient failure means "try again in 60 seconds", you must clear the claim, or the lease outlives the backoff and silently becomes the real retry interval:

def schedule_retry(message, delay)
  message.update!(
    next_attempt_at: Time.current + delay,
    claimed_until:   nil,   # without this, a 10-minute lease beats a 60s backoff
  )
end

I shipped a version without that line. The retry ladder was configured as 60s, 120s, 240s, 480s — and every one of those first four rungs was actually ten minutes, because claiming required both an expired lease and a due retry time. The ladder looked configured and wasn't. A test caught it; nothing in production would have, because "the email went out eventually" is not an alert.

Make idempotence survive the send. Claim-then-send is at-most-once per lease, not exactly-once. If the process dies after the provider accepted the message but before you recorded it, the lease expires and you send twice. If that matters, pass an idempotency key the provider deduplicates on.

Lesson 3: a queue tier exists when someone else owns the latency

Most advice about queue topology is "put important things on a fast queue". That's the wrong axis, and it produces a critical queue with everything in it.

The useful question is: who controls how long this takes?

• Work that is pure database and CPU — you control the latency. It is bounded and predictable.
• Work that calls someone else's API, SMTP server, or webhook receiver — they control the latency. It is unbounded and correlated: when a provider is having a bad day, every job of that kind stalls at once.

Mixing the two is what kills a pool. This system had a three-thread queue carrying per-minute reminder dispatchers, webhook fan-out, workflow execution and a batch sender that looped over recipients at up to 60 seconds each. Two large batches took two of the three threads, and every reminder in the system queued behind a mail provider.

The split that works:

# Cheap, ours, must be prompt — fan-out that decides an event EXISTS
class WebhookFanoutJob < ApplicationJob
  queue_as :critical        # milliseconds of DB writes
end

# Expensive, theirs — the actual delivery to a receiver we don't control
class WebhookDeliveryJob < ApplicationJob
  queue_as :webhooks        # 8-second read timeout against a stranger
end

One customer's hanging endpoint now exhausts the webhooks pool and nothing else. The fan-out keeps recording events at full speed.

Two rules fall out of this:

A slow tier should be isolated, not prioritised. The goal isn't for delivery to be fast — you can't make someone else's server fast. The goal is for its slowness to be contained.

Don't re-tune pool sizes at the same time you move a job. Moving a job between queues is a correctness change you can reason about. Resizing pools is a capacity question that needs measured queue latency. I deliberately did the first and refused the second, because the original mis-sized configuration came from exactly that kind of confident guess.

And pin it in a test, because queue assignment is one attribute on one line and nothing else notices when it changes:

it "keeps third-party delivery off the tier that must stay responsive" do
  expect(WebhookDeliveryJob.new.queue_name).to eq("webhooks")
  expect(WebhookFanoutJob.new.queue_name).to eq("critical")
end

Lesson 4: fan out per tenant instead of looping over tenants

Seven cron jobs had this shape:

# DON'T
def perform
  Tenant.find_each do |tenant|
    RecomputeService.call(tenant: tenant)
  end
end

Three things are wrong with it, and the third is the worst:

1. Runtime is O(all tenants × their data) on one thread. A nightly recompute pinned half a pool for its whole duration.
2. No fairness. Tenant #1 is always freshest; the last tenant is always stalest.
3. One tenant's exception ends the sweep for everyone after it. GoodJob v4 does not retry unhandled errors — a job that raises is discarded. So one bad row didn't delay the night's work, it deleted it, for every tenant sorted after the failure.

That third one produced a real, long-lived bug: a score that read zero for a large slice of tenants, for weeks, because a single tenant raised early in the ordering every night.

The retrofit people reach for is a rescue inside the loop. That's a patch on the shape, not a fix of it — it converts a loud failure into a silent one and leaves problems 1 and 2 untouched.

Fan out instead. One job per tenant:

module TenantSweep
  extend ActiveSupport::Concern

  included do
    include GoodJob::ActiveJobExtensions::Concurrency

    # One rule serving both roles. The fan-out key coalesces overlapping cron
    # ticks; the per-tenant key stops one tenant being swept twice at once.
    good_job_control_concurrency_with(
      total_limit: 1,
      key: -> {
        arg = arguments.first
        tid = arg.is_a?(Hash) ? (arg[:tenant_id] || arg["tenant_id"]) : nil
        tid ? "#{self.class.name}-tenant-#{tid}" : "#{self.class.name}-fanout"
      },
    )
  end

  # No args   -> fan out, do no real work.
  # tenant_id -> do the work for exactly one tenant.
  def perform(tenant_id: nil)
    return sweep_one(tenant_id) if tenant_id

    sweep_scope.find_each(batch_size: 200) do |tenant|
      self.class.perform_later(tenant_id: tenant.id)
    end
  end

  private

  def sweep_one(tenant_id)
    tenant = Tenant.find_by(id: tenant_id)
    return unless tenant   # deleted between fan-out and pickup

    sweep_tenant(tenant)   # let this RAISE — see below
  end

  # Override to narrow which tenants get a job at all.
  def sweep_scope = Tenant.all
end

An includer is now three lines:

class RecomputeScoresJob < ApplicationJob
  queue_as :bulk
  include TenantSweep

  private

  def sweep_tenant(tenant)
    RecomputeService.call(tenant: tenant)
  end
end

Deliberately, sweep_tenant is not rescued. A per-tenant job that fails is visible in the dashboard and the error tracker with tenant context, and it fails alone. That is strictly better than an aggregate that logs a number.

One real cost to be honest about: you now write a job row per tenant per tick. For a 15-minute sweep across many tenants with a week of job retention, that adds up. So narrow the scope in SQL when most tenants are no-ops:

def sweep_scope
  # Only tenants that actually have something to scan.
  Tenant.where(id: Project.active.select(:tenant_id))
end

Lesson 5: job arguments are a credential store

This one is a security lesson wearing a performance lesson's clothes.

# DON'T
ImportJob.perform_later(import_id, api_token)

The comment above it said "we don't persist the token". ActiveJob serialises every argument into the job row. Postgres-backed queues keep finished rows for a retention window — a week, in this case — and that table is not encrypted and is readable from the mounted admin dashboard.

So a third-party API token sat in cleartext in a database table for seven days after each import, and the code said it didn't.

The fix is a small wrapper whose serialised form is ciphertext:

class SecretArgument
  def self.wrap(value)   = new(value)
  def self.reveal(value) = value.is_a?(SecretArgument) ? value.to_s : value

  # ...paired with an ActiveJob::Serializers::ObjectSerializer that encrypts
  # in #serialize and decrypts in #deserialize.
end

# enqueue side
ImportJob.perform_later(import_id, SecretArgument.wrap(api_token))

# job side
def perform(import_id, token)
  token = SecretArgument.reveal(token)
  # ...
end

Then stop it coming back with a test that greps enqueue sites, because the next person to add a job will not have read this:

it "no job is enqueued with a credential-shaped bare argument" do
  offenders = enqueue_sites_matching(/perform_later\(.*(token|secret|password|api_key)/)
                .reject { |site| site.include?("SecretArgument.wrap") }

  expect(offenders).to be_empty
end

While you're there, check what your error tracker receives. A base job class that attaches arguments to every report will happily ship the same secrets to a third party:

around_perform do |job, block|
  ErrorTracker.context(
    job_class: job.class.name,
    # Hashes and arrays are the shapes that carry payloads and credentials.
    # Replace them rather than serialising them; truncate the rest.
    job_args: job.arguments.map { |a|
      a.is_a?(Hash) || a.is_a?(Array) ? "[redacted]" : a.to_s.first(120)
    }
  )
  block.call
end

Lesson 6: the Postgres timeout you're probably missing

Most teams set two:

variables:
  statement_timeout: 60s   # cap on ONE statement
  lock_timeout:      10s   # cap on WAITING for a lock

There's a third, and it catches something neither of the others can:

  idle_in_transaction_session_timeout: 60s

A session that has run BEGIN, executed something, and then gone quiet is not running a statement, so statement_timeout never fires. Meanwhile it still holds its locks and still pins the xmin horizon — the VACUUM-blocking problem from Lesson 2, except now it can last forever, because nothing is counting.

That is exactly what a transaction wrapped around a hung HTTP call looks like.

Two things to know before you enable it:

It kills the session, not the statement. Your application sees a connection failure, not a timeout:

ActiveRecord::ConnectionFailed:
  PQconsumeInput() FATAL: terminating connection due to
  idle-in-transaction timeout

Not QueryCanceled. It reads like a network blip, and someone will lose an afternoon to that if it isn't written down somewhere.

Audit your transactions first. Enabling this while any code path holds a transaction across a slow non-database operation converts a latent problem into a user-facing error. I enumerated every transaction do block before turning it on — which brings me to the lesson I actually value most.

Lesson 7: a guard you haven't tried to break isn't a guard

Every fix above shipped with a test. Every one of those tests I then deliberately broke, by reverting the fix and confirming the test failed.

That step caught more problems than the original review did.

A test that passed for the wrong reason. I wrote a spec asserting a watchdog wouldn't reset a freshly started job. It passed. Then I reverted the fix and it still passed — because a library's own hook was producing the behaviour I was attributing to my code. My test asserted a dependency's behaviour, not mine. I deleted it rather than leave false assurance in the suite.

A static analyser that found almost nothing, confidently. I wrote a script to enumerate transaction do blocks and check their contents. It reported 2 blocks, 0 risky. The real number was 127. My regex was:

/[^\w.]transaction\s+do/   # written to "avoid matching the dot"

The negated . threw out every ActiveRecord::Base.transaction do — 110 of the 127. A scan that finds nothing passes every check you build on top of it, so any tool like this needs a floor:

expect(blocks.size).to be >= 100,
  "only #{blocks.size} matched; there were 127 when this was written. " \
  "A scan that finds nothing passes every other assertion here."

Comments that look exactly like code. With the regex fixed, the scan flagged ten blocks for shell execution. Every single one was a comment containing backticked prose, matching a backtick-command pattern. Strip comments before you match:

def strip_comments(lines)
  lines.reject { |l| l =~ /\A\s*#/ }
       .map    { |l| l.sub(/#(?![{$@]).*$/, "") }  # trailing, but not interpolation
       .join
end

After both fixes: 127 blocks, 0 doing non-database work inside a transaction. Three different answers from the same script, and only the third was true.

The mutation loop itself can be vacuous. My first attempt applied mutations with ruby -i -pe, which died on a multibyte character in the source and left the file truncated. Every test "failed", so every mutation looked caught. The runs proved nothing. Assert the mutation actually landed before trusting the result:

mutated = original.sub(from, to)
raise "mutation did not apply — anchor not found" if mutated == original

If you take one thing from this article, take this: a test you have never seen fail is a test you have never verified. Reverting the fix and watching the test go red takes thirty seconds, and it is the only evidence that the test is attached to the behaviour you think it is.

The meta-lesson: audit documents rot faster than code

I wrote a findings document at the start. By the end, three of its claims were wrong — and I only found out because I re-checked each one against the code before calling it done, rather than against my own notes.

• One finding said a set of classes needed consolidating. They already shared a base class. I'd counted files in a directory instead of reading the inheritance.
• One finding said retention volume was slowing the hot path. The hot path used partial indexes (WHERE finished_at IS NULL), so retained rows weren't in them at all. The cost was disk and dashboard pagination, not throughput — a completely different priority.
• One finding said to bound a loop. The loop was over a small, indexed table. The real cost was five unbounded aggregates above it, one joining with an OR across two polymorphic branches — a shape no index serves, and one that bounding the loop would not have touched.

Two of those "fixes" would have been pure motion. The third would have been worse than nothing: it would have closed the item without touching the problem, and the document would then have said the problem was solved.

There's a related trap I walked into. Partway through, I reported a phase complete. It wasn't — two items were still open, and I'd conflated a transaction fix on some jobs with the queue placement fix that was actually specified. Nobody would have caught that except me, re-reading the plan against the tree.

Verify against the artefact, not against your memory of the artefact. That applies to your own work most of all, because that's where you are most confident and least skeptical.

What I'd do first, on any job system

If you inherit a job queue and want the highest-value hour:

1. Read your concurrency library's actual query and error path. Not the option names. You are probably not guarding what you think.
2. Grep for transaction and look for network calls inside. HTTP, SMTP, object storage — anything with a socket. Claim-commit-then-send each one.
3. Sort your jobs by who owns the latency, and make sure yours and theirs don't share a thread pool.
4. Look at what's in your job arguments table, and at what your error tracker receives.
5. Pick your three most complex jobs and write a test for each. Then break the code and confirm the test notices.

Step 5 is the one people skip, and it's the one that tells you whether the other four actually worked.