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

Extend bidirectional API to use raft server id in the destination_t type #69

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

raakella1
Copy link
Contributor

No description provided.


if (std::holds_alternative< svr_id_t >(dest)) {
if (auto const id_str = id_to_str(std::get< svr_id_t >(dest)); !id_str.empty()) {
return boost::uuids::string_generator()(id_str);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not very clear how this can get the uuid for corresponding originator's uuid. Is it because the way we generate server_id (take some portion of uuid), that it can be used to call this api to get its uuid?

Copy link
Collaborator

@szmyd szmyd Feb 2, 2024

Choose a reason for hiding this comment

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

nuraft::raft_server::get_config()->get_server(...) which is used to implement id_to_str gets the pair which is srv_id, endpoint. Endpoint is an opaque string to nuraft, but we store the string version of the uuid; so we just need to convert it back to boost::uuid here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The id_to_str() method [defined here https://github.com/eBay/nuraft_mesg/blob/main/src/lib/repl_service_ctx.cpp#L15] returns the std::string form of the uuid from the endpoint field

Copy link
Collaborator

@szmyd szmyd left a comment

Choose a reason for hiding this comment

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

This looks okay to me assuming the UT coverage comes back good.

@yamingk yamingk merged commit 48913b1 into eBay:main Feb 2, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants