-
Notifications
You must be signed in to change notification settings - Fork 59
feat(split): add splitting_replicas while on_config_sync #653
Changes from 9 commits
a1b9da5
f219068
98fbc65
1f84e2b
d427304
fab8732
b7c61e8
f9c2883
090d0c6
ec63550
1f5cb05
7a22859
ea97b48
607c609
6a4aae0
3f4cd47
d653e7b
aba615d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1324,10 +1324,13 @@ void replica_stub::on_node_query_reply(error_code err, | |
return; | ||
} | ||
|
||
ddebug("process query node partitions response for resp.err = ERR_OK, " | ||
"partitions_count(%d), gc_replicas_count(%d)", | ||
(int)resp.partitions.size(), | ||
(int)resp.gc_replicas.size()); | ||
auto splitting_count = resp.__isset.splitting_replicas ? resp.splitting_replicas.size() : 0; | ||
|
||
ddebug_f("process query node partitions response for resp.err = ERR_OK, " | ||
"partitions_count({}), gc_replicas_count({}), splitting_replicas({})", | ||
resp.partitions.size(), | ||
resp.gc_replicas.size(), | ||
splitting_count); | ||
|
||
replicas rs; | ||
{ | ||
|
@@ -1336,11 +1339,20 @@ void replica_stub::on_node_query_reply(error_code err, | |
} | ||
|
||
for (auto it = resp.partitions.begin(); it != resp.partitions.end(); ++it) { | ||
auto meta_split_status = split_status::NOT_SPLIT; | ||
if (splitting_count > 0) { | ||
auto iter = resp.splitting_replicas.find(it->config.pid); | ||
if (iter != resp.splitting_replicas.end()) { | ||
meta_split_status = iter->second; | ||
} | ||
} | ||
rs.erase(it->config.pid); | ||
tasking::enqueue(LPC_QUERY_NODE_CONFIGURATION_SCATTER, | ||
&_tracker, | ||
std::bind(&replica_stub::on_node_query_reply_scatter, this, this, *it), | ||
it->config.pid.thread_hash()); | ||
tasking::enqueue( | ||
LPC_QUERY_NODE_CONFIGURATION_SCATTER, | ||
&_tracker, | ||
std::bind( | ||
&replica_stub::on_node_query_reply_scatter, this, this, *it, meta_split_status), | ||
it->config.pid.thread_hash()); | ||
} | ||
|
||
// for rps not exist on meta_servers | ||
|
@@ -1377,7 +1389,11 @@ void replica_stub::set_meta_server_connected_for_test( | |
for (auto it = resp.partitions.begin(); it != resp.partitions.end(); ++it) { | ||
tasking::enqueue(LPC_QUERY_NODE_CONFIGURATION_SCATTER, | ||
&_tracker, | ||
std::bind(&replica_stub::on_node_query_reply_scatter, this, this, *it), | ||
std::bind(&replica_stub::on_node_query_reply_scatter, | ||
this, | ||
this, | ||
*it, | ||
split_status::NOT_SPLIT), | ||
it->config.pid.thread_hash()); | ||
} | ||
} | ||
|
@@ -1393,11 +1409,12 @@ void replica_stub::set_replica_state_subscriber_for_test(replica_state_subscribe | |
// replica_stub::close | ||
// ThreadPool: THREAD_POOL_REPLICATION | ||
void replica_stub::on_node_query_reply_scatter(replica_stub_ptr this_, | ||
const configuration_update_request &req) | ||
const configuration_update_request &req, | ||
split_status::type meta_split_status) | ||
{ | ||
replica_ptr replica = get_replica(req.config.pid); | ||
if (replica != nullptr) { | ||
replica->on_config_sync(req.info, req.config); | ||
replica->on_config_sync(req.info, req.config, meta_split_status); | ||
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's ugly to particularly add an argument for one requirement. It makes the function responsibility obscure. I'd prefer adding Think about it, dozens of parameters in a function, if we keep doing that. And I admit this code is shit, so we can't make it worse. 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. Done |
||
} else { | ||
if (req.config.primary == _primary_address) { | ||
ddebug("%s@%s: replica not exists on replica server, which is primary, remove 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.
maybe you can calculate
meta_split_status
in meta server. So we don't need to calculate it in each replica serverThere 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.
If an app is not splitting,
splitting_replicas
inconfiguration_query_by_node_response
won't be set. The config_sync rpc will happen every 30 seconds for all replica servers, and partition split is not a frequent action, for most cases it will reduce the network cost.