-
Notifications
You must be signed in to change notification settings - Fork 97
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
add members_info state query and API wrapper #413
Conversation
Opening for review. @kjnilsson what if we called it Second question, should I send zeroes instead of no values for the ease of matching? This mainly concerns the corner case of a local query to non-leader. On the flip side, I fear sending anything invalid will lead to user confusion. |
@illotum |
src/ra_server_proc.erl
Outdated
Res -> | ||
Res | ||
end | ||
_ -> aten:register(Node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a direct equivalent (maybe this does not matter). In the original version a value of ignore
returned by Aten would be transformed into an ok
by Ra. Now that's not the case, is there a reason for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dialyze mostly! aten:register
returns only ok
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you have an old aten version, see https://github.com/rabbitmq/aten/blob/main/src/aten.erl#L18
Added a commit to extract membership from Eh. This breaks dyalize again, I'll revert back to full voter_status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of smaller changes needed but looks good.
Also would you mind running an indentation pass on the new code in ra_server_proc. It's all a bit wonky :)
src/ra_server_proc.erl
Outdated
Res -> | ||
Res | ||
end | ||
_ -> aten:register(Node) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you have an old aten version, see https://github.com/rabbitmq/aten/blob/main/src/aten.erl#L18
I think the link to
I think what we're hitting here is the fact that this query makes less sense as a local query as really you want to issue it against the leader. I suggest when used as a local query we only return infos that make sense to return, e.g. the |
I'm leaning towards returning something. There are cases where one may want to investigate follower's view of the cluster, however limited that is. And I'll specify in the function doc that local queries are a special case. |
In the context of RabbitMQ's needs of determining if a node hosts any "catching up" (non-voter) replicas, either can work but a leader query makes more sense to me. |
Proposed Changes
Adds state query and API to return leader's cluster view.
Types of Changes
Checklist
CONTRIBUTING.md
document