-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implement file-based leader election method #90
base: master
Are you sure you want to change the base?
Changes from all commits
3347d33
98e49b4
659b219
6137800
05b092d
32ad158
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
// (C) 2024 Cybozu. | ||
|
||
#include "election.hpp" | ||
#include "config.hpp" | ||
|
||
#include <filesystem> | ||
|
||
namespace fs = std::filesystem; | ||
|
||
namespace yrmcds { | ||
|
||
bool is_master() { | ||
auto method = g_config.leader_election_method(); | ||
if( method == leader_election_method::virtual_ip ) { | ||
auto vip = g_config.vip(); | ||
return cybozu::has_ip_address(*vip); | ||
} else if( method == leader_election_method::file ) { | ||
auto& path = *g_config.master_file_path(); | ||
return fs::exists(path); | ||
} else { | ||
throw std::runtime_error("Invalid leader_election_method"); | ||
} | ||
} | ||
|
||
} // namespace yrmcds |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Leader election. | ||
// (C) 2024 Cybozu. | ||
|
||
#ifndef YRMCDS_ELECTION_HPP | ||
#define YRMCDS_ELECTION_HPP | ||
|
||
namespace yrmcds { | ||
|
||
bool is_master(); | ||
|
||
} // namespace yrmcds | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,9 +73,12 @@ void handler::on_start() { | |
make_server_socket(nullptr, g_config.port(), w), | ||
cybozu::reactor::EVENT_IN); | ||
} else { | ||
m_reactor.add_resource( | ||
make_server_socket(g_config.vip(), g_config.port(), w, true), | ||
cybozu::reactor::EVENT_IN); | ||
if( g_config.vip() ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. This is always true, IIUC. |
||
auto vip = g_config.vip()->str(); | ||
m_reactor.add_resource( | ||
make_server_socket(vip.c_str(), g_config.port(), w, true), | ||
cybozu::reactor::EVENT_IN); | ||
} | ||
for( auto& s: g_config.bind_ip() ) { | ||
m_reactor.add_resource( | ||
make_server_socket(s, g_config.port(), w), | ||
|
@@ -95,8 +98,9 @@ void handler::on_master_start() { | |
make_server_socket(nullptr, g_config.repl_port(), w), | ||
cybozu::reactor::EVENT_IN); | ||
} else { | ||
auto master_host = g_config.master_host(); | ||
m_reactor.add_resource( | ||
make_server_socket(g_config.vip(), g_config.repl_port(), w, true), | ||
make_server_socket(master_host.c_str(), g_config.repl_port(), w, true), | ||
cybozu::reactor::EVENT_IN); | ||
for( auto& s: g_config.bind_ip() ) { | ||
m_reactor.add_resource( | ||
|
@@ -145,12 +149,27 @@ void handler::on_master_end() { | |
} | ||
|
||
bool handler::on_slave_start() { | ||
int fd = cybozu::tcp_connect(g_config.vip().str().c_str(), | ||
g_config.repl_port()); | ||
using logger = cybozu::logger; | ||
|
||
auto master_host = g_config.master_host(); | ||
int fd; | ||
try { | ||
fd = cybozu::tcp_connect(master_host.c_str(), g_config.repl_port()); | ||
} catch( std::runtime_error& err ) { | ||
logger::error() << "Failed to connect to the master (" << master_host << "): " << err.what(); | ||
m_reactor.run_once(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does this intend to do? A comment would be helpful. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was called in the existing error handling, so I called it in the code I added. if( fd == -1 ) {
m_reactor.run_once();
return false;
} I'm not aware of the original intent of the code, so I'm just guessing: since signal handling, etc., depends on the reactor, we should sometimes run the reactor during re-connecting to the master. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. Thanks. |
||
return false; | ||
} | ||
if( fd == -1 ) { | ||
logger::error() << "Failed to connect to the master (" << master_host << ")"; | ||
m_reactor.run_once(); | ||
return false; | ||
} | ||
|
||
// on_slave_start may be called multiple times over the lifetime. | ||
// Therefore we need to clear the hash table. | ||
clear(); | ||
|
||
m_repl_client_socket = new repl_client_socket(fd, m_hash); | ||
m_reactor.add_resource(std::unique_ptr<cybozu::resource>(m_repl_client_socket), | ||
cybozu::reactor::EVENT_IN|cybozu::reactor::EVENT_OUT ); | ||
|
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 guess this is always true because
m_vip
is initialized to"127.0.0.1"
.Am I right?
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.
Oh, that's right.
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.
Since
m_vip
is always set, I think usingoptional
for this might be confusing and lead to bugs.I'll leave this to you to figure out how to fix it.
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.
When
leader_election_method
isfile
, yrmcds actually has no virtual IPs. Therefore, I feel that the type ofm_vip
should bestd::optional
to model the situation correctly.What is wrong is the initial value of
m_vip
. It should be initialized to benullopt
and should only have the user-specified value or127.0.0.1
when invirtual_ip
mode.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 was wrong.
Since the initial value of
leader_election_method
isvirtual_ip
, the initial value ofm_vip
must be127.0.0.1
.Therefore, we must empty
m_vip
whenleader_election_method
is changed to other thanvirtual_ip
.