Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rabbit_peer_discovery: Add pre/post discovery steps #10763

Closed
wants to merge 2 commits into from

Conversation

dumbbell
Copy link
Member

Why

The Consul peer discovery backend needs to create a session before it can acquire a lock. This session is also required for nodes to discover each other.

This session was created as part of the lock callback. However, after pull request #9797, the lock was only acquired if and when a node had ot join another one. Thus, after the actual discovery phase. This broke Consul peer discovery because the discovery was performed before that Consul session was created.

How

We introduce two new callbacks, pre_discovery/0 and post_discovery/1 to allow a backend to perform actions before and after the whole discover/lock/join process. The new callbacks are used by the Consul peer discovery backend to create and delete that session.

To remain compatible with other peer discovery backend, the new callbacks are optional.

Fixes #10760.

@dumbbell dumbbell self-assigned this Mar 18, 2024
[Why]
The Consul peer discovery backend needs to create a session before it
can acquire a lock. This session is also required for nodes to discover
each other.

This session was created as part of the lock callback. However, after
pull request #9797, the lock was only acquired if and when a node had to
join another one. Thus, after the actual discovery phase. This broke
Consul peer discovery because the discovery was performed before that
Consul session was created.

[How]
We introduce two new callbacks, `pre_discovery/0` and `post_discovery/1`
to allow a backend to perform actions before and after the whole
discover/lock/join process.

To remain compatible with other peer discovery backend, the new
callbacks are optional.
…callbacks

[Why]
The Consul peer discovery backend needs to create a session before it
can acquire a lock. This session is also required for nodes to discover
each other.

It must open the session before the `list_nodes/0` callback can return
meaningful results.

[How]
The new `pre_discovery/0` and `post_discovery/1` callbacks are used to
create and delete that session before the whole discover/lock/join
process.

Fixes #10760.
@dumbbell dumbbell force-pushed the peer-disc-add-setup-teardown-around-discovery branch from 496cfde to 33dc0f2 Compare March 18, 2024 14:28
@lukebakken lukebakken self-requested a review March 18, 2024 16:26
@dumbbell dumbbell added the bug label Mar 19, 2024
@dumbbell dumbbell added this to the 4.0.0 milestone Mar 19, 2024
@michaelklishin michaelklishin added bug and removed bug labels Mar 19, 2024
@dumbbell
Copy link
Member Author

dumbbell commented May 3, 2024

This approch was superseded by #11045.

@dumbbell dumbbell closed this May 3, 2024
@dumbbell dumbbell deleted the peer-disc-add-setup-teardown-around-discovery branch May 3, 2024 13:58
@dumbbell dumbbell removed this from the 4.0.0 milestone May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RabbitMQ 3.13.0 nodes with Consul peer discovery enabled fails to form a cluster
2 participants