Handle timeouts possible in Khepri minority in rabbit_db_exchange (backport #11785)
#11821
Conversation
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
(cherry picked from commit 80f599b)
A common case for exchange deletion is that callers want the deletion
to be idempotent: they treat the `ok` and `{error, not_found}` returns
from `rabbit_exchange:delete/3` the same way. To simplify these
callsites we add a `rabbit_exchange:ensure_deleted/3` that wraps
`rabbit_exchange:delete/3` and returns `ok` when the exchange did not
exist. Part of this commit is to update callsites to use this helper.
The other part is to handle the `rabbit_khepri:timeout()` error possible
when Khepri is in a minority. For most callsites this is just a matter
of adding a branch to their `case` clauses and an appropriate error and
message.
(cherry picked from commit e7489d2)
It's unlikely that these operations will time out since the serial number is always updated after some other transaction, for example adding or deleting an exchange. In the future we could consider moving the serial updates into those transactions. In the meantime we can remove the possibility of timeouts by giving the serial update unlimited time to finish. (cherry picked from commit 7059582)
(cherry picked from commit 96c60a2)
The spec of `rabbit_exchange:declare/7` needs to be updated to return
`{ok, Exchange} | {error, Reason}` instead of the old return value of
`rabbit_types:exchange()`. This is safe to do since `declare/7` is not
called by RPC - from the CLI or otherwise - outside of test suites, and
in test suites only through the CLI's `TestHelper.declare_exchange/7`.
Callers of this helper are updated in this commit.
Otherwise this commit updates callers to unwrap the `{ok, Exchange}`
and bubble up errors.
(cherry picked from commit 52a0d70)
`rabbit_amqp_management` returns HTTP status codes to the client. 503 means that a service is unavailable (which Khepri is while it is in a minority) so it's a more appropriate code than the generic 500 internal server error. (cherry picked from commit 98616a0)
Co-authored-by: David Ansari <[email protected]> (cherry picked from commit fb3154b)
This fixes a potential crash in `rabbit_amqp_amanegment` where we tried to format the exchange resource as a string (`~ts`). The other changes are cosmetic. (cherry picked from commit b56abee)
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.
Continuation of the changes like in #11685 and #11706 but for exchanges.
The changes for exchanges are a bit more involved than prior PRs. Here's a summary of the changes:
rabbit_exchange:declare/7's spec is updated to return{ok, #exchange{}} | {error, timeout}where it returned just#exchange{}beforerabbit_exchange:delete/3need some small changes to handle an{error, timeout}returnrabbit_exchange:ensure_deleted/3wrapper that covers the common case for callers whereok | {error, not_found}are handled the same way.timeout => infinityfor calls for updating the exchange serialThis is an automatic backport of pull request Handle timeouts possible in Khepri minority in
rabbit_db_exchange#11785 done by Mergify.