Skip to content

fix: avoid nested RabbitMQ pub channel acquire on tenant register - #4433

Merged
grutt merged 2 commits into
mainfrom
fix/rabbitmq-pub-register-tenant-double-acquire
Jul 15, 2026
Merged

fix: avoid nested RabbitMQ pub channel acquire on tenant register#4433
grutt merged 2 commits into
mainfrom
fix/rabbitmq-pub-register-tenant-double-acquire

Conversation

@grutt

@grutt grutt commented Jul 15, 2026

Copy link
Copy Markdown
Member

Description

Avoid nested RabbitMQ pub-channel acquires when registering a tenant exchange during publish. Under concurrent load (e.g. top-of-hour crons), holding one pooled channel and then acquiring a second exhausted the pool and timed out, dropping cron triggers.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

What's Changed

  • Declare tenant exchange on the already-held publish channel in pubMessage
  • Extract declareTenantExchange for shared use by RegisterTenant and pubMessage

Checklist

Changes have been:

  • Tested (unit, integration, or manually with steps specified)
  • Linted and formatted
  • Documented (where applicable)
  • Added to CHANGELOG (where applicable) -- see Keep a Changelog

Testing

  • go build ./internal/msgqueue/rabbitmq/
  • golangci-lint via pre-commit
  • After deploy: confirm top-of-hour controllers no longer spike on [RegisterTenant] cannot acquire channel / could not run cron workflow

🤖 AI Disclosure
  • I acknowledge that an LLM was used in the creation of this Pull Request, in accordance with Hatchet's AI_POLICY.md.

  • Details: Investigating production cron misses via o11y, identifying nested channel acquire, implementing fix and PR

pubMessage held a pooled publish channel and then called RegisterTenant,
which acquired a second channel. Under top-of-hour cron bursts this
exhausted the pub pool and timed out, dropping cron triggers.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hatchet-docs Ready Ready Preview, Comment Jul 15, 2026 1:19pm

Request Review

@github-actions github-actions Bot added the engine Related to the core Hatchet engine label Jul 15, 2026
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Benchmark results

goos: linux
goarch: amd64
pkg: github.com/hatchet-dev/hatchet/internal/msgqueue/rabbitmq
cpu: AMD Ryzen 9 7950X3D 16-Core Processor          
                              │ /tmp/old.txt │             /tmp/new.txt             │
                              │    sec/op    │    sec/op      vs base               │
CompressPayloads_1x10KiB-8      98.59µ ±  8%    80.66µ ± 17%  -18.19% (p=0.009 n=6)
CompressPayloads_10x10KiB-8     1.280m ± 11%    1.012m ±  7%  -20.95% (p=0.002 n=6)
CompressPayloads_10x100KiB-8    14.94m ±  7%    13.36m ± 19%        ~ (p=0.485 n=6)
CompressPayloads_Concurrent-8   78.09µ ±  5%   104.97µ ± 30%  +34.42% (p=0.002 n=6)
geomean                         619.4µ          581.7µ         -6.10%

                              │ /tmp/old.txt │            /tmp/new.txt            │
                              │     B/op     │     B/op      vs base              │
CompressPayloads_1x10KiB-8      10.97Ki ± 1%   10.96Ki ± 1%       ~ (p=0.952 n=6)
CompressPayloads_10x10KiB-8     108.6Ki ± 1%   109.0Ki ± 1%       ~ (p=1.000 n=6)
CompressPayloads_10x100KiB-8    2.922Mi ± 0%   2.921Mi ± 0%       ~ (p=0.485 n=6)
CompressPayloads_Concurrent-8   54.53Ki ± 1%   54.46Ki ± 1%       ~ (p=0.589 n=6)
geomean                         118.1Ki        118.1Ki       +0.05%

                              │ /tmp/old.txt │            /tmp/new.txt            │
                              │  allocs/op   │ allocs/op   vs base                │
CompressPayloads_1x10KiB-8        5.000 ± 0%   5.000 ± 0%       ~ (p=1.000 n=6) ¹
CompressPayloads_10x10KiB-8       32.00 ± 0%   32.00 ± 0%       ~ (p=1.000 n=6) ¹
CompressPayloads_10x100KiB-8      63.00 ± 2%   63.00 ± 2%       ~ (p=1.000 n=6)
CompressPayloads_Concurrent-8     17.00 ± 0%   17.00 ± 0%       ~ (p=1.000 n=6) ¹
geomean                           20.35        20.35       +0.00%
¹ all samples are equal

Compared against main (2ea5926)

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Optional test failure: The load-deadlock job failed on this PR (optimistic-scheduling=true). This check is non-mandatory and does not block merging, but may be worth investigating. View logs

Comment thread internal/msgqueue/rabbitmq/rabbitmq.go Outdated
Comment on lines +476 to +478
// Reuse the already-held publish channel. Calling RegisterTenant here would
// Acquire a second pub channel while this one is still checked out and can
// exhaust the pool under concurrent publish load (e.g. top-of-hour crons).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove unnecessary comment

@grutt
grutt merged commit cf1a9f0 into main Jul 15, 2026
50 of 51 checks passed
@grutt
grutt deleted the fix/rabbitmq-pub-register-tenant-double-acquire branch July 15, 2026 13:15
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Optional test failure: The load-deadlock job failed on this PR (optimistic-scheduling=true). This check is non-mandatory and does not block merging, but may be worth investigating. View logs

mrshu added a commit to mrshu/sre-world that referenced this pull request Jul 25, 2026
Add the graded scenario for the fault that hatchet-dev/hatchet#4433 fixed.
The publish path keeps one channel from the pool. It then takes a second
channel from the same pool to register the tenant exchange. Under load the
pool becomes empty and the service loses messages.

The fault ships as a Tier-2 layer image. The layer compiles the publish
service from the source before the fix, and it sets the pool size to 2 so
the fault appears quickly. The healthy base image keeps the merged fix. A
layer fault carries no chart overlay, so both the mechanism and the pool
size live in the image.

The repair is an operator action: change the image of svc-pub back to the
healthy base. Two gates grade the result. The outcome gate sees the lost
messages. The image_state gate proves the rollback happened. The second
gate is necessary, because a larger channel pool hides the lost messages
while the faulted code stays in the service.

The scenario is publication_pending. The images are not published, so the
generator writes no task and the scenario is not in tasks/. The bands are
provisional until a calibration runs on a cluster.

- Add spec.yaml, ground-truth.yaml, instruction.md, solve.sh and the fault
  layer with its patch
- pub.pub-pool is the decoy in the closed component inventory. It is the
  place where the symptom appears, and not the cause.
mrshu added a commit to mrshu/sre-world that referenced this pull request Jul 25, 2026
Add a standalone reproduction of the fault from hatchet-dev/hatchet#4433.
One command starts it. The reproduction runs the true Hatchet message queue
against a true RabbitMQ broker, at the pinned commit 245c06e. It changes
only two controls. It sets the pub channel pool to 2, which the Hatchet
option WithMaxPubChannels supports, and it applies a patch that puts back
the code from before the fix. An external load generator built on the
shared arrival engine applies the load.

Measured result at a pool of 2, with the same load for both runs. The armed
build loses 291 of 324 messages and the log shows 291 "cannot acquire
channel" errors. The fixed build loses none.

verify_dual_gate.py proves that the two gates of the scenario separate the
three cases, and it does so without a cluster. It runs three episodes and
grades each one with the true oracle against the committed answer key. It
states the expected result for each episode and exits with an error if the
gates stop separating them.

  faulted image, small pool   outcome FAIL, image_state FAIL   FAIL
  faulted image, larger pool  outcome PASS, image_state FAIL   FAIL
  healthy image after rollback  outcome PASS, image_state PASS  PASS

The second case is the reason the image_state gate exists. A larger pool
hides the lost messages, so every gate for the symptom passes. Only the
gate for the cause finds that the faulted code is still in the service.

The scripts provision the broker user through the image entrypoint and then
verify the credential. An earlier version added the user after the broker
started and did not check the result. On a slow start that command failed
in silence, every publish returned 403, and both runs then showed a total
loss that looked the same as the incident.
mrshu added a commit to mrshu/sre-world that referenced this pull request Jul 25, 2026
Add the graded scenario for the fault that hatchet-dev/hatchet#4433 fixed.
The publish path keeps one channel from the pool. It then takes a second
channel from the same pool to register the tenant exchange. Under load the
pool becomes empty and the service loses messages.

The fault ships as a Tier-2 layer image. The layer compiles the publish
service from the source before the fix, and it sets the pool size to 2 so
the fault appears quickly. The healthy base image keeps the merged fix. A
layer fault carries no chart overlay, so both the mechanism and the pool
size live in the image.

The repair is an operator action: change the image of svc-pub back to the
healthy base. Two gates grade the result. The outcome gate sees the lost
messages. The image_state gate proves the rollback happened. The second
gate is necessary, because a larger channel pool hides the lost messages
while the faulted code stays in the service.

The scenario is publication_pending. The images are not published, so the
generator writes no task and the scenario is not in tasks/. The bands are
provisional until a calibration runs on a cluster.

- Add spec.yaml, ground-truth.yaml, instruction.md, solve.sh and the fault
  layer with its patch
- pub.pub-pool is the decoy in the closed component inventory. It is the
  place where the symptom appears, and not the cause.
mrshu added a commit to mrshu/sre-world that referenced this pull request Jul 25, 2026
Add a standalone reproduction of the fault from hatchet-dev/hatchet#4433.
One command starts it. The reproduction runs the true Hatchet message queue
against a true RabbitMQ broker, at the pinned commit 245c06e. It changes
only two controls. It sets the pub channel pool to 2, which the Hatchet
option WithMaxPubChannels supports, and it applies a patch that puts back
the code from before the fix. An external load generator built on the
shared arrival engine applies the load.

Measured result at a pool of 2, with the same load for both runs. The armed
build loses 291 of 324 messages and the log shows 291 "cannot acquire
channel" errors. The fixed build loses none.

verify_dual_gate.py proves that the two gates of the scenario separate the
three cases, and it does so without a cluster. It runs three episodes and
grades each one with the true oracle against the committed answer key. It
states the expected result for each episode and exits with an error if the
gates stop separating them.

  faulted image, small pool   outcome FAIL, image_state FAIL   FAIL
  faulted image, larger pool  outcome PASS, image_state FAIL   FAIL
  healthy image after rollback  outcome PASS, image_state PASS  PASS

The second case is the reason the image_state gate exists. A larger pool
hides the lost messages, so every gate for the symptom passes. Only the
gate for the cause finds that the faulted code is still in the service.

The scripts provision the broker user through the image entrypoint and then
verify the credential. An earlier version added the user after the broker
started and did not check the result. On a slow start that command failed
in silence, every publish returned 403, and both runs then showed a total
loss that looked the same as the incident.
mrshu added a commit to abundant-ai/sre-world that referenced this pull request Jul 28, 2026
Add the graded scenario for the fault that hatchet-dev/hatchet#4433 fixed.
The publish path keeps one channel from the pool. It then takes a second
channel from the same pool to register the tenant exchange. Under load the
pool becomes empty and the service loses messages.

The fault ships as a Tier-2 layer image. The layer compiles the publish
service from the source before the fix, and it sets the pool size to 2 so
the fault appears quickly. The healthy base image keeps the merged fix. A
layer fault carries no chart overlay, so both the mechanism and the pool
size live in the image.

The repair is an operator action: change the image of svc-pub back to the
healthy base. Two gates grade the result. The outcome gate sees the lost
messages. The image_state gate proves the rollback happened. The second
gate is necessary, because a larger channel pool hides the lost messages
while the faulted code stays in the service.

The scenario is publication_pending. The images are not published, so the
generator writes no task and the scenario is not in tasks/. The bands are
provisional until a calibration runs on a cluster.

- Add spec.yaml, ground-truth.yaml, instruction.md, solve.sh and the fault
  layer with its patch
- pub.pub-pool is the decoy in the closed component inventory. It is the
  place where the symptom appears, and not the cause.
mrshu added a commit to abundant-ai/sre-world that referenced this pull request Jul 28, 2026
Add a standalone reproduction of the fault from hatchet-dev/hatchet#4433.
One command starts it. The reproduction runs the true Hatchet message queue
against a true RabbitMQ broker, at the pinned commit 245c06e. It changes
only two controls. It sets the pub channel pool to 2, which the Hatchet
option WithMaxPubChannels supports, and it applies a patch that puts back
the code from before the fix. An external load generator built on the
shared arrival engine applies the load.

Measured result at a pool of 2, with the same load for both runs. The armed
build loses 291 of 324 messages and the log shows 291 "cannot acquire
channel" errors. The fixed build loses none.

verify_dual_gate.py proves that the two gates of the scenario separate the
three cases, and it does so without a cluster. It runs three episodes and
grades each one with the true oracle against the committed answer key. It
states the expected result for each episode and exits with an error if the
gates stop separating them.

  faulted image, small pool   outcome FAIL, image_state FAIL   FAIL
  faulted image, larger pool  outcome PASS, image_state FAIL   FAIL
  healthy image after rollback  outcome PASS, image_state PASS  PASS

The second case is the reason the image_state gate exists. A larger pool
hides the lost messages, so every gate for the symptom passes. Only the
gate for the cause finds that the faulted code is still in the service.

The scripts provision the broker user through the image entrypoint and then
verify the credential. An earlier version added the user after the broker
started and did not check the result. On a slow start that command failed
in silence, every publish returned 403, and both runs then showed a total
loss that looked the same as the incident.
mrshu added a commit to abundant-ai/sre-world that referenced this pull request Jul 28, 2026
Add the graded scenario for the fault that hatchet-dev/hatchet#4433 fixed.
The publish path keeps one channel from the pool. It then takes a second
channel from the same pool to register the tenant exchange. Under load the
pool becomes empty and the service loses messages.

The fault ships as a Tier-2 layer image. The layer compiles the publish
service from the source before the fix, and it sets the pool size to 2 so
the fault appears quickly. The healthy base image keeps the merged fix. A
layer fault carries no chart overlay, so both the mechanism and the pool
size live in the image.

The repair is an operator action: change the image of svc-pub back to the
healthy base. Two gates grade the result. The outcome gate sees the lost
messages. The image_state gate proves the rollback happened. The second
gate is necessary, because a larger channel pool hides the lost messages
while the faulted code stays in the service.

The scenario is publication_pending. The images are not published, so the
generator writes no task and the scenario is not in tasks/. The bands are
provisional until a calibration runs on a cluster.

- Add spec.yaml, ground-truth.yaml, instruction.md, solve.sh and the fault
  layer with its patch
- pub.pub-pool is the decoy in the closed component inventory. It is the
  place where the symptom appears, and not the cause.
mrshu added a commit to abundant-ai/sre-world that referenced this pull request Jul 28, 2026
Add a standalone reproduction of the fault from hatchet-dev/hatchet#4433.
One command starts it. The reproduction runs the true Hatchet message queue
against a true RabbitMQ broker, at the pinned commit 245c06e. It changes
only two controls. It sets the pub channel pool to 2, which the Hatchet
option WithMaxPubChannels supports, and it applies a patch that puts back
the code from before the fix. An external load generator built on the
shared arrival engine applies the load.

Measured result at a pool of 2, with the same load for both runs. The armed
build loses 291 of 324 messages and the log shows 291 "cannot acquire
channel" errors. The fixed build loses none.

verify_dual_gate.py proves that the two gates of the scenario separate the
three cases, and it does so without a cluster. It runs three episodes and
grades each one with the true oracle against the committed answer key. It
states the expected result for each episode and exits with an error if the
gates stop separating them.

  faulted image, small pool   outcome FAIL, image_state FAIL   FAIL
  faulted image, larger pool  outcome PASS, image_state FAIL   FAIL
  healthy image after rollback  outcome PASS, image_state PASS  PASS

The second case is the reason the image_state gate exists. A larger pool
hides the lost messages, so every gate for the symptom passes. Only the
gate for the cause finds that the faulted code is still in the service.

The scripts provision the broker user through the image entrypoint and then
verify the credential. An earlier version added the user after the broker
started and did not check the result. On a slow start that command failed
in silence, every publish returned 403, and both runs then showed a total
loss that looked the same as the incident.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine Related to the core Hatchet engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants