-
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 4 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 |
---|---|---|
|
@@ -51,9 +51,12 @@ void handler::on_master_start() { | |
make_server_socket(nullptr, port, w), | ||
cybozu::reactor::EVENT_IN); | ||
} else { | ||
m_reactor.add_resource( | ||
make_server_socket(g_config.vip(), 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. I guess this is always true because 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. Oh, that's right. 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. Since 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 commentThe reason will be displayed to describe this comment to others. Learn more. When What is wrong is the initial value of 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. ↑This was wrong. Since the initial value of |
||
auto vip = g_config.vip()->str(); | ||
m_reactor.add_resource( | ||
make_server_socket(vip.c_str(), port, w, true), | ||
cybozu::reactor::EVENT_IN); | ||
} | ||
for( auto& s: g_config.bind_ip() ) { | ||
m_reactor.add_resource( | ||
make_server_socket(s, port, w), | ||
|
@@ -83,6 +86,10 @@ void handler::on_master_end() { | |
m_gc_thread = nullptr; // join | ||
} | ||
|
||
bool handler::on_slave_start() { | ||
clear(); | ||
} | ||
|
||
void handler::dump_stats() { | ||
std::uint64_t ops = 0; | ||
for( auto& v: g_stats.ops ) { | ||
|
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.
Shouldn't we initialize
m_leader_election_method
here as well?It looks inconsistent to initialize the field elsewhere.
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.
m_leader_election_method
is initialized with the following line: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 know. I thought it was inconsistent.
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 will move all member initializations to the declaration statement.
I think it is easier to read when they are grouped together rather than separated in the constructor and member declarations.