PAYMENTS-11727 Deliver metrics recorded inside Resque jobs - #45
Draft
WillemHoman wants to merge 12 commits into
Draft
PAYMENTS-11727 Deliver metrics recorded inside Resque jobs#45WillemHoman wants to merge 12 commits into
WillemHoman wants to merge 12 commits into
Conversation
added 3 commits
August 8, 2026 14:18
added 9 commits
August 8, 2026 16:11
…its for an in-flight send
…e in the parent before each fork
… overhead across Resque forks
…easuring changes by hand
WillemHoman
force-pushed
the
PAYMENTS-11727-resque_latency_metrics_clear_queue
branch
from
August 8, 2026 07:04
6a2efb4 to
fc80632
Compare
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira: PAYMENTS-11727
Replaces #42, which was opened on a misspelled branch and closes as won't-merge.
What? Why?
Metrics recorded inside a Resque job were being dropped. Bigpay released
0.8.3withPROMETHEUS_RESQUE_PER_JOB_METRICS_ENABLED=1andruby_webhooks_published_counterfell to about 8% of normal on everyworker pod, recovering exactly on rollback. No webhooks were lost, no jobs failed and the queue never backed up. Only
the recording broke.
The fix everyone gets: the child no longer inherits the parent's queue
The client is a singleton and its queue is ordinary process memory, so
forkcopies it.JobMetrics.record_queue_latencyenqueues in the parent on the line immediately beforesuper, which is the fork, soevery child inherited that message and had to re-send it, and anything else pending, before reaching its own. That
moved the survival threshold from about 2ms of post-record life to 25-50ms, and the webhook job sat between the two.
A
Resque.after_forkhook now hands each child a clean client. Nothing is lost: the parent still holds the originalsand sends them on its own schedule. Unconditional, no configuration, no cost. This is the incident fix, and it is
the reason to take the bump even if you read no further.
The opt-in part: delivering what a job records
Recording only queues; delivery happens on a background thread that wakes every
client_thread_sleepseconds. A Resquechild ends with
exit!, which runs no at_exit handlers and kills threads outright, so an observation recorded near theend of a job is destroyed with the child. That predates
0.8.3and has always been costing observations quietly.A prepend on
Resque::Worker#perform, the in-child boundary, drains on the calling thread before the job returns.Off by default. It costs one request per job that records something, and bumping this gem should not change how long
anybody's jobs take. Jobs that record nothing pay nothing either way. Enable with
PROMETHEUS_RESQUE_CHILD_FLUSH_ENABLED=1, or by assigningresque_child_flush_enabled.That default is a position rather than caution waiting to be undone: turning it on for everyone would change other
people's job latency, which is breaking and wants a version bump to match, as
0.4.0did when the thread pool defaultwent from 20 to 3.
An empty queue is not an empty wire
Found by the bench below, on its first run. The flush originally returned early when nothing was queued, and the queue
reads as empty the instant the background thread pops the last message, well before it reaches the collector.
exit!then destroyed the request: roughly 0.1% to 0.3% of in-child records, silently, because the process that would have
logged it was already gone.
Confirmed at the socket layer, where the listener accepted every connection but read fewer complete requests, and the
shortfall matched its count of connections opened and then closed with no request line. Both drain paths are now
serialised on a delivery mutex, so a flush cannot return while a send is in flight. That also closes a hang where both
threads saw one queued message, both called
pop, and the loser blocked forever.A job should not wait long on the metrics pipeline
Making the child wait for delivery means it now waits for a collector that is not answering, and
0.8.3has no suchexposure because nothing in a job's code path ever touched the network. Delivery is bounded by
PROMETHEUS_CLIENT_FLUSH_TIMEOUT, 20ms by default, covering the wait for the lock as well as the requests. Past it theobservations are abandoned, because availability of the work beats completeness of its metrics, and a warning says how
many.
Turning it on and off without a restart
resque_child_flush_enabledalso accepts anything callable, asked in the parent before every fork. The childinherits the answer through the fork, so a feature flag client never has to survive one, and nothing in a child ever
evaluates anything:
A callable taking an argument receives the
Resque::Job, so a rollout can be gated per job as well as per process.Anything it raises is treated as "do not flush", since it runs as a
Resque.before_forkhook where an escapingexception would stop the worker.
The env var supplies the default and an assignment overrides it, as with all 30 settings here, so a callable replaces
the env var rather than layering on top of it.
Also in here
Net::HTTPdefaults all three to 60 seconds,survivable on a background thread and not survivable inline in a job.
bin/resque_fork_bench, a manual tool that forks real children and reports both what arrived and what it cost, witha
--smoke-testsweep and--collector healthy|stalled|down. Every measurement below came from it.resqueandsinatra >= 4.0as dev dependencies, the work deferred from PAYMENTS-11727 Resque latency metrics #31. Resque pulls sinatra with a looseconstraint and, with no
Gemfile.lockhere, a cold CI resolve was free to pick a sinatra cappingrack < 3againstthe gemspec's
rack >= 3.0.confirmed during an incident rather than inferred.
How was it tested?
spec/bigcommerce/prometheus/client_spec.rbcovers#flush!and#reset_after_fork!: delivery on the callingthread, never raising into the caller, waiting for an in-flight send, giving up on the deadline, bounded timeouts on
both paths, and clearing of the queue, worker thread, both mutexes and socket state. One example binds a real socket
that accepts and never answers, because a timeout cannot be asserted against a stub.
spec/bigcommerce/prometheus/integrations/resque_spec.rbcovers resolving the setting in the parent: a plainvalue, an arity-0 callable, an arity-1 callable receiving the job, an object with a
#callmethod, and a raisingcallable that returns false without propagating.
spec/bigcommerce/prometheus/integrations/resque/child_flush_spec.rbcovers the in-child wrapper: flushes when theparent enabled it, does not when it did not, does not on a non-forking worker, and still flushes when the job raises.
spec/integration/resque_fork_delivery_spec.rbis black box, forking real Resque children against a real listener.It asserts two properties and deliberately mentions neither queues nor forks, because past changes have broken each
from opposite directions:
cancels out.
Excluded from the default run and given its own job,
ruby-3_4-rspec_fork_integration. Therubyexecutor alreadyprovides redis, so no service needed adding.
None of it is vacuous, and each was checked by removing the fix:
< 1.0arityraisesNoMethodErrorand is swallowed< 25msMeasurements
The default is genuinely off, and the env var genuinely reaches the gem. 40 jobs recording one metric each:
PROMETHEUS_RESQUE_CHILD_FLUSH_ENABLED=1Cost of serialising delivery, A/B on the same machine, medians of five runs at one push and three at five:
Roughly half a millisecond on the shape bigpay runs. That is the calling thread paying for a request it previously
handed to a thread that was about to be killed.
Cost when the collector is not healthy, 50 jobs recording one metric each:
The last row is what the per-request timeouts alone would give. A dead collector costs nothing, since
ECONNREFUSEDisimmediate. Only a saturated one is expensive, and that is what the bound exists for.
The live switch, one process, 40 forks, callable flipped at job 20:
Delivery followed the flag mid-run with no restart. The probe records and returns immediately, so nothing but the flush
could have delivered those 20.
Ruled out: was the collector simply overloaded?
Worth asking, because enabling per-job metrics makes the parent record two extra envelopes per job, so the endpoint
takes roughly three times the requests, and the fork reset does nothing about that.
sum(rate(ruby_collector_sessions_total{job="bigpay-worker"}[5m]))peaked at 85.7 requests per second across thewhole fleet, against 60 worker exporter instances, each with its own Puma pool of three threads. Under two
requests per second each. Even if one instance had absorbed the entire fleet's traffic it would still be under a tenth
of one thread. And
ruby_collector_workingreads 1 across all 46,252 samples, minimum and maximum identical.On rollout,
ruby_collector_sessions_totalshould roughly triple, from about one request per job to three. That rise isthe feature working. It is a symptom only if
ruby_collector_workingleaves 1 orruby_collector_bad_metrics_totalstarts climbing.
Proof against the two prior regressions
Two draft PRs run these specs against the releases they exist to catch, one property failing on each. Both are expected
to be red and are not for merge. Both need rebasing onto this branch name:
0.8.3as released. Completeness fails at 0 of 100, overhead passes. This incident's signature: job timestayed flat while the observations went nowhere.
the observations arrived, they were just unaffordable.
Not covered
The specs cannot show loss caused by overloading the collector.
CountingExporteris a bareTCPServerwith a threadper connection, so it absorbs far more than the real exporter. Acceptable at the rates above, and if that changes the
answer is to bind a real
Bigcommerce::Prometheus::Serverwith a counting type collector rather than to teach the fakeone to imitate a thread pool.
A connection-reuse optimisation was written and measured, then dropped: the difference was inside run-to-run noise.