Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(split): register child partition #391

Merged
merged 26 commits into from
Apr 24, 2020

Conversation

hycdong
Copy link
Contributor

@hycdong hycdong commented Feb 11, 2020

Simple partition split process

  1. meta receives client partition split request, and change partition count split: meta start partition split #286
  2. replica notices partition count changed during on_config_sync
  3. parent partition create child partition split: parent replica create child replica #291
  4. parent prepare states for child to learn feat(split): parent replica prepare states #299
  5. child partition async learn states from parent feat(split): child replica learn parent prepare list and checkpoint #309 feat(split): child replica apply private logs, in-memory mutations and catch up parent #319
  6. child notify parent catch up feat(split): child notify parent catch up #390
  7. update child group partition count
  8. register child partition
  9. parent update group partition count and recover read and write

More partition split discussion in issue #69 and partition split design doc
This pr solves the part of fifth step of partition split, which is bold in process description.

What this pr solved

  • replica::register_child_on_meta: primary parent register child partition on meta_server and parent will reject read write client.
  • meta_split_service::register_child_on_meta: meta server update child partition_configuration structure on remote storage and local.
  • child_partition_active: when child registered, child of primary parent will be the primary of the child replica group, its status will turn from PS_PARTITION_SPLIT to PS_PRIMARY, child of secondary will update its status by group_check.

@hycdong hycdong marked this pull request as ready for review March 20, 2020 07:58
@hycdong hycdong changed the title feat(split): register child partition [WIP] feat(split): register child partition Mar 20, 2020
acelyc111
acelyc111 previously approved these changes Apr 16, 2020
src/dist/replication/lib/replica_split.cpp Show resolved Hide resolved
src/dist/replication/lib/replica_split.cpp Outdated Show resolved Hide resolved
src/dist/replication/lib/replica_split.cpp Show resolved Hide resolved
src/dist/replication/lib/replica_split.cpp Outdated Show resolved Hide resolved
Comment on lines +726 to +728
}

if (err == ERR_OK) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
if (err == ERR_OK) {
} else {

Copy link
Contributor Author

@hycdong hycdong Apr 17, 2020

Choose a reason for hiding this comment

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

L752-L759 will always be executed, you can see comments in L751

Copy link
Contributor

Choose a reason for hiding this comment

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

I see no comment:

// ThreadPool: THREAD_POOL_REPLICATION
void replica::child_partition_active(const partition_configuration &config) // on child
{
ddebug_replica("child partition become active");
_primary_states.last_prepare_decree_on_new_primary = _prepare_list->max_decree();
update_configuration(config);
}

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 line number changed after I wrote this comment.

// parent register child succeed or child partition has already resgitered
// in both situation, we should reset resgiter child task and child_gpid

const std::string &partition_path = _state->get_partition_path(request.child_config.pid);
blob value = dsn::json::json_forwarder<partition_configuration>::encode(request.child_config);
if (create_new) {
return _meta_svc->get_remote_storage()->create_node(
Copy link
Contributor

Choose a reason for hiding this comment

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

use get_meta_storage() instead, which can auto-repeat on timeout, and assert on unexpected error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to use get_meta_storage(), but I should handle ERR_TIMEOUT and ERR_NODE_ALREADY_EXIST specifically in callback function.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. I will later write a utility of meta_storage for this case.

register_child_rpc rpc(std::move(request), RPC_CM_REGISTER_CHILD_REPLICA);
split_svc().register_child_on_meta(rpc);
wait_all();
if (wait_zk) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's wait_zk used for? wait_all supposed to wait any tasks to complete.

Copy link
Contributor Author

@hycdong hycdong Apr 17, 2020

Choose a reason for hiding this comment

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

wait_all function will wait meta_service tasks, zk tasks are not included. The simple way is to sleep for several seconds to wait updating zk succeed.

@@ -369,6 +370,8 @@ void replica::update_configuration_on_meta_server(config_type::type type,
"");
dassert(
newConfig.primary == node, "%s VS %s", newConfig.primary.to_string(), node.to_string());
} else if (type == config_type::CT_REGISTER_CHILD) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let me describe this code in pseudocode:

If the type is not CT_PRIMARY_FORCE_UPDATE_BALLOT, then it cannot be CT_REGISTER_CHILD.

What does it suppose to mean? Can you add a comment here? Is that CT_REGISTER_CHILD is impossible, or not-CT_PRIMARY_FORCE_UPDATE_BALLOT && CT_REGISTER_CHILD is impossible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no relationship between CT_PRIMARY_FORCE_UPDATE_BALLOT and CT_REGISTER_CHILD. I have updated the code.

Comment on lines +726 to +728
}

if (err == ERR_OK) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see no comment:

// ThreadPool: THREAD_POOL_REPLICATION
void replica::child_partition_active(const partition_configuration &config) // on child
{
ddebug_replica("child partition become active");
_primary_states.last_prepare_decree_on_new_primary = _prepare_list->max_decree();
update_configuration(config);
}

const std::string &partition_path = _state->get_partition_path(request.child_config.pid);
blob value = dsn::json::json_forwarder<partition_configuration>::encode(request.child_config);
if (create_new) {
return _meta_svc->get_remote_storage()->create_node(
Copy link
Contributor

Choose a reason for hiding this comment

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

OK. I will later write a utility of meta_storage for this case.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants