Skip to content

A few related fixes around binding deletion - #17257

Merged
michaelklishin merged 5 commits into
mainfrom
rabbitmq-server-17255
Sep 2, 2026
Merged

A few related fixes around binding deletion#17257
michaelklishin merged 5 commits into
mainfrom
rabbitmq-server-17255

Conversation

@michaelklishin

@michaelklishin michaelklishin commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

These changes started with a fix for what #17255 describes but some deeper digging found a couple of remaining comparable problems.

This should be safe to backport to v4.3.x without a feature flag.

@michaelklishin michaelklishin added this to the 4.4.0 milestone Aug 28, 2026
@michaelklishin michaelklishin mentioned this pull request Aug 28, 2026
Closed
@mergify

mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@michaelklishin
michaelklishin marked this pull request as draft August 28, 2026 06:17
@michaelklishin

michaelklishin commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

@dumbbell my first attempt turned out to have a regression, so I've left a comment in the part that had to be revisited.

@michaelklishin
michaelklishin marked this pull request as ready for review August 29, 2026 02:49
@michaelklishin
rabbit_db_exchange: collect binding deletions before the record delete
56944b6 
With `tie_binding_to_dest_with_keep_while_cond` enabled, deleting the
exchange record first makes Khepri cascade delete the routes where the
exchange is the destination, so the subsequent binding query finds
nothing: no `remove_bindings/3` callback, no `binding.deleted` events.

Fixes #17255
@michaelklishin
rabbit_db_queue: preserve binding deletions in transient queue cleanup
6828061 
The queue record delete cascades the routes where the queue is the
destination. Derive the deletions from the delete's return value, using
a new transaction-safe variant of `node_props_to_deletions/2` (renamed
from `khepri_ret_to_deletions/2`).

References #17255
@michaelklishin
Virtual host deletion: preserve exchange-to-exchange binding deletions
bc78a38 
The exchange subtree delete cascades all exchange-to-exchange routes
before the fold can collect them. Derive the deletions from the
delete's return value.

References #17255
@michaelklishin
rabbit_db_binding: attach the keep_while condition to auto-delete…
5dc55c7 
… exchanges on bind

With the `tie_binding_to_dest_with_keep_while_cond` feature flag enabled,
an auto-delete exchange is deleted by a `keep_while` condition
on its record that requires at least one binding where it is the source.

The feature flag migration skips exchanges without such bindings (#16823),
and nothing attaches the condition later, so an exchange skipped this way
is never auto-deleted once it gains and then loses bindings.

Attach the condition when a binding is created, in the same
transaction: the new route satisfies it, and the re-put changes nothing
for exchanges that already carry it.

References #16823.
@michaelklishin

michaelklishin commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

The forced push was a rebase on top of main to benefit from a few flake fixes from #17280.

dumbbell
dumbbell reviewed Sep 1, 2026
View reviewed changes
Comment thread deps/rabbit/src/rabbit_db_exchange.erl Resolved
michaelklishin added a commit that referenced this pull request Sep 2, 2026
@michaelklishin
rabbit_db_binding: preserve exchange deletions cascaded by `keep_wh…
8ae03ec 
…ile`

Deleting the routes of a destination can remove an auto-delete source
exchange's last binding, so Khepri deletes that exchange in the same
`khepri_tx_adv:delete_many/1` call.

`maybe_auto_delete_exchange_in_khepri/4` then finds no record and adds
nothing: no `exchange.deleted`, no `binding.deleted` events.

Derive them from the delete's return value as well.

`rabbit_db_exchange:delete_in_khepri/3` now does the same for the
exchange record delete, per JSP's suggestion in #17257.

Older Khepri machine versions fall back to the previous flow: collect
the bindings first, then delete the record.
michaelklishin added a commit that referenced this pull request Sep 2, 2026
@michaelklishin
rabbit_db_binding: preserve exchange deletions cascaded by `keep_wh…
2dbc0f6 
…ile`

Deleting the routes of a destination can remove an auto-delete source
exchange's last binding, so Khepri deletes that exchange in the same
`khepri_tx_adv:delete_many/1` call.

`maybe_auto_delete_exchange_in_khepri/4` then finds no record and adds
nothing: no `exchange.deleted`, no `binding.deleted` events.

Derive them from the delete's return value as well.

`rabbit_db_exchange:delete_in_khepri/3` now does the same for the
exchange record delete, per JSP's suggestion in #17257.

Older Khepri machine versions fall back to the previous flow: collect
the bindings first, then delete the record.
michaelklishin added a commit that referenced this pull request Sep 2, 2026
@michaelklishin
rabbit_db_binding: preserve exchange deletions cascaded by `keep_wh…
9268aa9 
…ile`

Deleting the routes of a destination can remove an auto-delete source
exchange's last binding, so Khepri deletes that exchange in the same
`khepri_tx_adv:delete_many/1` call.

`maybe_auto_delete_exchange_in_khepri/4` then finds no record and adds
nothing: no `exchange.deleted`, no `binding.deleted` events.

Derive them from the delete's return value as well.

`rabbit_db_exchange:delete_in_khepri/3` now does the same for the
exchange record delete, per JSP's suggestion in #17257.

Older Khepri machine versions fall back to the previous flow: collect
the bindings first, then delete the record.
@michaelklishin

michaelklishin commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

With the most recent implementation, you can notice that some delete operations will no no-ops in practice because their "deletion targets" are removed by the underlying functions.

I intentionally left that as instead of continuing the refactoring. These no-ops are cheap and safe, and the goal of this PR is to fix a few remaining bugs in that corner of the codebase, not refactor it in a major way.

@michaelklishin
rabbit_db_binding: preserve exchange deletions cascaded by `keep_wh…
ba84296 
…ile`

Deleting the routes of a destination can remove an auto-delete source
exchange's last binding, so Khepri deletes that exchange in the same
`khepri_tx_adv:delete_many/1` call.

`maybe_auto_delete_exchange_in_khepri/4` then finds no record and adds
nothing: no `exchange.deleted`, no `binding.deleted` events.

Derive them from the delete's return value as well.

`rabbit_db_exchange:delete_in_khepri/3` now does the same for the
exchange record delete, per JSP's suggestion in #17257.

Older Khepri machine versions fall back to the previous flow: collect
the bindings first, then delete the record.
@michaelklishin
michaelklishin merged commit 57dd518 into main Sep 2, 2026
188 checks passed
@michaelklishin
michaelklishin deleted the rabbitmq-server-17255 branch September 2, 2026 02:45
@mergify mergify Bot mentioned this pull request Sep 2, 2026
Merged
michaelklishin added a commit that referenced this pull request Sep 2, 2026
@michaelklishin
Merge pull request #17290 from rabbitmq/mergify/bp/v4.3.x/pr-17257
36972ec 
A few related fixes around binding deletion (backport #17257)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dumbbell dumbbell dumbbell left review comments

Assignees

No one assigned

Labels

backport-v4.3.x

Projects

None yet

Milestone

4.4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@michaelklishin @dumbbell