From c18949e3e20d647b94ec2ab27f807d471e7bef23 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 30 Aug 2021 16:24:52 +0800 Subject: [PATCH 1/4] refactor: move construct_replica from simple_load_balancer to meta_data --- src/meta/meta_data.cpp | 67 +++++++++++ src/meta/meta_data.h | 8 ++ src/meta/server_load_balancer.cpp | 67 ----------- src/meta/server_load_balancer.h | 11 -- src/meta/server_state.cpp | 4 +- src/meta/test/meta_data.cpp | 116 +++++++++++++++++++ src/meta/test/meta_load_balance_test.cpp | 118 -------------------- src/meta/test/update_configuration_test.cpp | 4 - 8 files changed, 193 insertions(+), 202 deletions(-) diff --git a/src/meta/meta_data.cpp b/src/meta/meta_data.cpp index 5ced4115eb..3e6c2b9d49 100644 --- a/src/meta/meta_data.cpp +++ b/src/meta/meta_data.cpp @@ -78,6 +78,73 @@ void maintain_drops(std::vector &drops, const rpc_address &node, co when_update_replicas(t, action); } +bool construct_replica(meta_view view, const gpid &pid, int max_replica_count) +{ + partition_configuration &pc = *get_config(*view.apps, pid); + config_context &cc = *get_config_context(*view.apps, pid); + + dassert(replica_count(pc) == 0, + "replica count of gpid(%d.%d) must be 0", + pid.get_app_id(), + pid.get_partition_index()); + dassert( + max_replica_count > 0, "max replica count is %d, should be at lease 1", max_replica_count); + + std::vector &drop_list = cc.dropped; + if (drop_list.empty()) { + dwarn("construct for (%d.%d) failed, coz no replicas collected", + pid.get_app_id(), + pid.get_partition_index()); + return false; + } + + // treat last server in drop_list as the primary + const dropped_replica &server = drop_list.back(); + dassert(server.ballot != invalid_ballot, + "the ballot of server must not be invalid_ballot, node = %s", + server.node.to_string()); + pc.primary = server.node; + pc.ballot = server.ballot; + pc.partition_flags = 0; + pc.max_replica_count = max_replica_count; + + ddebug("construct for (%d.%d), select %s as primary, ballot(%" PRId64 + "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", + pid.get_app_id(), + pid.get_partition_index(), + server.node.to_string(), + server.ballot, + server.last_committed_decree, + server.last_prepared_decree); + + drop_list.pop_back(); + + // we put max_replica_count-1 recent replicas to last_drops, in case of the DDD-state when the + // only primary dead + // when add node to pc.last_drops, we don't remove it from our cc.drop_list + dassert(pc.last_drops.empty(), + "last_drops of partition(%d.%d) must be empty", + pid.get_app_id(), + pid.get_partition_index()); + for (auto iter = drop_list.rbegin(); iter != drop_list.rend(); ++iter) { + if (pc.last_drops.size() + 1 >= max_replica_count) + break; + // similar to cc.drop_list, pc.last_drop is also a stack structure + pc.last_drops.insert(pc.last_drops.begin(), iter->node); + ddebug("construct for (%d.%d), select %s into last_drops, ballot(%" PRId64 + "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", + pid.get_app_id(), + pid.get_partition_index(), + iter->node.to_string(), + iter->ballot, + iter->last_committed_decree, + iter->last_prepared_decree); + } + + cc.prefered_dropped = (int)drop_list.size() - 1; + return true; +} + proposal_actions::proposal_actions() : from_balancer(false) { reset_tracked_current_learner(); } void proposal_actions::reset_tracked_current_learner() diff --git a/src/meta/meta_data.h b/src/meta/meta_data.h index 070cbf6519..9abfce48e6 100644 --- a/src/meta/meta_data.h +++ b/src/meta/meta_data.h @@ -538,6 +538,14 @@ void maintain_drops(/*inout*/ std::vector &drops, const dsn::rpc_address &node, config_type::type t); +// +// Try to construct a replica-group by current replica-infos of a gpid +// ret: +// if construct the replica successfully, return true. +// Notice: as long as we can construct something from current infos, we treat it as a +// success +bool construct_replica(meta_view view, const gpid &pid, int max_replica_count); + inline bool has_seconds_expired(uint64_t second_ts) { return second_ts * 1000 < dsn_now_ms(); } inline bool has_milliseconds_expired(uint64_t milliseconds_ts) diff --git a/src/meta/server_load_balancer.cpp b/src/meta/server_load_balancer.cpp index 2b4eacbac5..7a329c9e7d 100644 --- a/src/meta/server_load_balancer.cpp +++ b/src/meta/server_load_balancer.cpp @@ -956,72 +956,5 @@ bool simple_load_balancer::collect_replica(meta_view view, return info.status == partition_status::PS_POTENTIAL_SECONDARY || ans != -1; } - -bool simple_load_balancer::construct_replica(meta_view view, const gpid &pid, int max_replica_count) -{ - partition_configuration &pc = *get_config(*view.apps, pid); - config_context &cc = *get_config_context(*view.apps, pid); - - dassert(replica_count(pc) == 0, - "replica count of gpid(%d.%d) must be 0", - pid.get_app_id(), - pid.get_partition_index()); - dassert( - max_replica_count > 0, "max replica count is %d, should be at lease 1", max_replica_count); - - std::vector &drop_list = cc.dropped; - if (drop_list.empty()) { - dwarn("construct for (%d.%d) failed, coz no replicas collected", - pid.get_app_id(), - pid.get_partition_index()); - return false; - } - - // treat last server in drop_list as the primary - const dropped_replica &server = drop_list.back(); - dassert(server.ballot != invalid_ballot, - "the ballot of server must not be invalid_ballot, node = %s", - server.node.to_string()); - pc.primary = server.node; - pc.ballot = server.ballot; - pc.partition_flags = 0; - pc.max_replica_count = max_replica_count; - - ddebug("construct for (%d.%d), select %s as primary, ballot(%" PRId64 - "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", - pid.get_app_id(), - pid.get_partition_index(), - server.node.to_string(), - server.ballot, - server.last_committed_decree, - server.last_prepared_decree); - - drop_list.pop_back(); - - // we put max_replica_count-1 recent replicas to last_drops, in case of the DDD-state when the - // only primary dead - // when add node to pc.last_drops, we don't remove it from our cc.drop_list - dassert(pc.last_drops.empty(), - "last_drops of partition(%d.%d) must be empty", - pid.get_app_id(), - pid.get_partition_index()); - for (auto iter = drop_list.rbegin(); iter != drop_list.rend(); ++iter) { - if (pc.last_drops.size() + 1 >= max_replica_count) - break; - // similar to cc.drop_list, pc.last_drop is also a stack structure - pc.last_drops.insert(pc.last_drops.begin(), iter->node); - ddebug("construct for (%d.%d), select %s into last_drops, ballot(%" PRId64 - "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", - pid.get_app_id(), - pid.get_partition_index(), - iter->node.to_string(), - iter->ballot, - iter->last_committed_decree, - iter->last_prepared_decree); - } - - cc.prefered_dropped = (int)drop_list.size() - 1; - return true; -} } // namespace replication } // namespace dsn diff --git a/src/meta/server_load_balancer.h b/src/meta/server_load_balancer.h index 40c13a5520..8d6c5c5917 100644 --- a/src/meta/server_load_balancer.h +++ b/src/meta/server_load_balancer.h @@ -161,15 +161,6 @@ class server_load_balancer virtual bool collect_replica(meta_view view, const dsn::rpc_address &node, const replica_info &info) = 0; - // - // Try to construct a replica-group by current replica-infos of a gpid - // ret: - // if construct the replica successfully, return true. - // Notice: as long as we can construct something from current infos, we treat it as a - // success - // - virtual bool construct_replica(meta_view view, const gpid &pid, int max_replica_count) = 0; - void register_proposals(meta_view view, const configuration_balancer_request &req, configuration_balancer_response &resp); @@ -308,8 +299,6 @@ class simple_load_balancer : public server_load_balancer bool collect_replica(meta_view view, const rpc_address &node, const replica_info &info) override; - bool construct_replica(meta_view view, const gpid &pid, int max_replica_count) override; - void register_ctrl_commands() override; void unregister_ctrl_commands() override; diff --git a/src/meta/server_state.cpp b/src/meta/server_state.cpp index 5b33a43e0b..f223e2e9e4 100644 --- a/src/meta/server_state.cpp +++ b/src/meta/server_state.cpp @@ -2088,8 +2088,8 @@ error_code server_state::construct_partitions( ddebug("ignore constructing partitions for dropping app(%d)", app->app_id); } else { for (partition_configuration &pc : app->partitions) { - bool is_succeed = _meta_svc->get_balancer()->construct_replica( - {&_all_apps, &_nodes}, pc.pid, app->max_replica_count); + bool is_succeed = + construct_replica({&_all_apps, &_nodes}, pc.pid, app->max_replica_count); if (is_succeed) { ddebug("construct partition(%d.%d) succeed: %s", app->app_id, diff --git a/src/meta/test/meta_data.cpp b/src/meta/test/meta_data.cpp index b4b4bbd221..2d8d370833 100644 --- a/src/meta/test/meta_data.cpp +++ b/src/meta/test/meta_data.cpp @@ -1,4 +1,5 @@ #include +#include "misc/misc.h" #include "meta/meta_data.h" using namespace dsn::replication; @@ -48,3 +49,118 @@ TEST(meta_data, dropped_cmp) ASSERT_TRUE(dropped_cmp(d2, d1) == 0); } } + +TEST(meta_data, construct_replica) +{ + app_mapper app; + node_mapper nodes; + + dsn::app_info info; + info.app_id = 1; + info.is_stateful = true; + info.status = dsn::app_status::AS_AVAILABLE; + info.app_name = "test"; + info.app_type = "test"; + info.max_replica_count = 3; + info.partition_count = 1024; + std::shared_ptr the_app = app_state::create(info); + app.emplace(the_app->app_id, the_app); + meta_view view = {&app, &nodes}; + + replica_info rep; + rep.app_type = "test"; + rep.pid = dsn::gpid(1, 0); + + dsn::partition_configuration &pc = *get_config(app, rep.pid); + config_context &cc = *get_config_context(app, rep.pid); + + std::vector node_list; + generate_node_list(node_list, 10, 10); + +#define CLEAR_REPLICA \ + do { \ + pc.primary.set_invalid(); \ + pc.secondaries.clear(); \ + pc.last_drops.clear(); \ + } while (false) + +#define CLEAR_DROP_LIST \ + do { \ + cc.dropped.clear(); \ + } while (false) + +#define CLEAR_ALL \ + CLEAR_REPLICA; \ + CLEAR_DROP_LIST + + // drop_list is empty, can't construct replica + { + CLEAR_ALL; + ASSERT_FALSE(construct_replica(view, rep.pid, 3)); + ASSERT_EQ(0, replica_count(pc)); + } + + // only have one node in drop_list + { + CLEAR_ALL; + cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 5, 10, 12}}; + ASSERT_TRUE(construct_replica(view, rep.pid, 3)); + ASSERT_EQ(node_list[0], pc.primary); + ASSERT_TRUE(pc.secondaries.empty()); + ASSERT_TRUE(cc.dropped.empty()); + ASSERT_EQ(-1, cc.prefered_dropped); + } + + // have multiple nodes, ballots are not same + { + CLEAR_ALL; + cc.dropped = {dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 6, 10, 12}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 10, 12}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 8, 10, 12}, + dropped_replica{node_list[4], dropped_replica::INVALID_TIMESTAMP, 9, 11, 12}}; + ASSERT_TRUE(construct_replica(view, rep.pid, 3)); + ASSERT_EQ(node_list[4], pc.primary); + ASSERT_TRUE(pc.secondaries.empty()); + + std::vector nodes = {node_list[2], node_list[3]}; + ASSERT_EQ(nodes, pc.last_drops); + ASSERT_EQ(3, cc.dropped.size()); + ASSERT_EQ(2, cc.prefered_dropped); + } + + // have multiple node, two have same ballots + { + CLEAR_ALL; + cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 5, 10, 12}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 7, 11, 12}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 12, 12}}; + + ASSERT_TRUE(construct_replica(view, rep.pid, 3)); + ASSERT_EQ(node_list[2], pc.primary); + ASSERT_TRUE(pc.secondaries.empty()); + + std::vector nodes = {node_list[0], node_list[1]}; + ASSERT_EQ(nodes, pc.last_drops); + ASSERT_EQ(2, cc.dropped.size()); + ASSERT_EQ(1, cc.prefered_dropped); + } + + // have multiple nodes, all have same ballots + { + CLEAR_ALL; + cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 7, 11, 14}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 7, 12, 14}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 13, 14}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 7, 14, 14}}; + + ASSERT_TRUE(construct_replica(view, rep.pid, 3)); + ASSERT_EQ(node_list[3], pc.primary); + ASSERT_TRUE(pc.secondaries.empty()); + + std::vector nodes = {node_list[1], node_list[2]}; + ASSERT_EQ(nodes, pc.last_drops); + + ASSERT_EQ(3, cc.dropped.size()); + ASSERT_EQ(2, cc.prefered_dropped); + } +} diff --git a/src/meta/test/meta_load_balance_test.cpp b/src/meta/test/meta_load_balance_test.cpp index 05e0526098..30e3f13b40 100644 --- a/src/meta/test/meta_load_balance_test.cpp +++ b/src/meta/test/meta_load_balance_test.cpp @@ -1169,124 +1169,6 @@ void meta_load_balance_test::simple_lb_collect_replica() #undef CLEAR_DROP_LIST } -void meta_load_balance_test::simple_lb_construct_replica() -{ - app_mapper app; - node_mapper nodes; - - dsn::app_info info; - info.app_id = 1; - info.is_stateful = true; - info.status = dsn::app_status::AS_AVAILABLE; - info.app_name = "test"; - info.app_type = "test"; - info.max_replica_count = 3; - info.partition_count = 1024; - std::shared_ptr the_app = app_state::create(info); - app.emplace(the_app->app_id, the_app); - meta_view view = {&app, &nodes}; - - replica_info rep; - rep.app_type = "test"; - rep.pid = dsn::gpid(1, 0); - - dsn::partition_configuration &pc = *get_config(app, rep.pid); - config_context &cc = *get_config_context(app, rep.pid); - - meta_service svc; - simple_load_balancer simple_lb(&svc); - - std::vector node_list; - generate_node_list(node_list, 10, 10); - -#define CLEAR_REPLICA \ - do { \ - pc.primary.set_invalid(); \ - pc.secondaries.clear(); \ - pc.last_drops.clear(); \ - } while (false) - -#define CLEAR_DROP_LIST \ - do { \ - cc.dropped.clear(); \ - } while (false) - -#define CLEAR_ALL \ - CLEAR_REPLICA; \ - CLEAR_DROP_LIST - - // drop_list is empty, can't construct replica - { - CLEAR_ALL; - ASSERT_FALSE(simple_lb.construct_replica(view, rep.pid, 3)); - ASSERT_EQ(0, replica_count(pc)); - } - - // only have one node in drop_list - { - CLEAR_ALL; - cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 5, 10, 12}}; - ASSERT_TRUE(simple_lb.construct_replica(view, rep.pid, 3)); - ASSERT_EQ(node_list[0], pc.primary); - ASSERT_TRUE(pc.secondaries.empty()); - ASSERT_TRUE(cc.dropped.empty()); - ASSERT_EQ(-1, cc.prefered_dropped); - } - - // have multiple nodes, ballots are not same - { - CLEAR_ALL; - cc.dropped = {dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 6, 10, 12}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 10, 12}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 8, 10, 12}, - dropped_replica{node_list[4], dropped_replica::INVALID_TIMESTAMP, 9, 11, 12}}; - ASSERT_TRUE(simple_lb.construct_replica(view, rep.pid, 3)); - ASSERT_EQ(node_list[4], pc.primary); - ASSERT_TRUE(pc.secondaries.empty()); - - std::vector nodes = {node_list[2], node_list[3]}; - ASSERT_EQ(nodes, pc.last_drops); - ASSERT_EQ(3, cc.dropped.size()); - ASSERT_EQ(2, cc.prefered_dropped); - } - - // have multiple node, two have same ballots - { - CLEAR_ALL; - cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 5, 10, 12}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 7, 11, 12}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 12, 12}}; - - ASSERT_TRUE(simple_lb.construct_replica(view, rep.pid, 3)); - ASSERT_EQ(node_list[2], pc.primary); - ASSERT_TRUE(pc.secondaries.empty()); - - std::vector nodes = {node_list[0], node_list[1]}; - ASSERT_EQ(nodes, pc.last_drops); - ASSERT_EQ(2, cc.dropped.size()); - ASSERT_EQ(1, cc.prefered_dropped); - } - - // have multiple nodes, all have same ballots - { - CLEAR_ALL; - cc.dropped = {dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 7, 11, 14}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 7, 12, 14}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 7, 13, 14}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 7, 14, 14}}; - - ASSERT_TRUE(simple_lb.construct_replica(view, rep.pid, 3)); - ASSERT_EQ(node_list[3], pc.primary); - ASSERT_TRUE(pc.secondaries.empty()); - - std::vector nodes = {node_list[1], node_list[2]}; - ASSERT_EQ(nodes, pc.last_drops); - - ASSERT_EQ(3, cc.dropped.size()); - ASSERT_EQ(2, cc.prefered_dropped); - } -} - TEST_F(meta_load_balance_test, simple_lb_balanced_cure) { simple_lb_balanced_cure(); } TEST_F(meta_load_balance_test, simple_lb_cure_test) { simple_lb_cure_test(); } diff --git a/src/meta/test/update_configuration_test.cpp b/src/meta/test/update_configuration_test.cpp index d69796713d..e3f16df383 100644 --- a/src/meta/test/update_configuration_test.cpp +++ b/src/meta/test/update_configuration_test.cpp @@ -113,10 +113,6 @@ class dummy_balancer : public dsn::replication::server_load_balancer { return false; } - virtual bool construct_replica(meta_view view, const dsn::gpid &pid, int max_replica_count) - { - return false; - } }; void meta_service_test_app::call_update_configuration( From 236ba9101c2879dc1d9e8b72ee94cd9dddd58bf1 Mon Sep 17 00:00:00 2001 From: levy Date: Mon, 30 Aug 2021 16:33:19 +0800 Subject: [PATCH 2/4] fix --- src/meta/test/meta_load_balance_test.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/meta/test/meta_load_balance_test.cpp b/src/meta/test/meta_load_balance_test.cpp index 30e3f13b40..4c303c7bbf 100644 --- a/src/meta/test/meta_load_balance_test.cpp +++ b/src/meta/test/meta_load_balance_test.cpp @@ -71,7 +71,6 @@ class meta_load_balance_test : public meta_test_base void simple_lb_balanced_cure(); void simple_lb_from_proposal_test(); void simple_lb_collect_replica(); - void simple_lb_construct_replica(); void call_update_configuration( meta_service *svc, std::shared_ptr &request) @@ -1177,7 +1176,5 @@ TEST_F(meta_load_balance_test, simple_lb_from_proposal_test) { simple_lb_from_pr TEST_F(meta_load_balance_test, simple_lb_collect_replica) { simple_lb_collect_replica(); } -TEST_F(meta_load_balance_test, simple_lb_construct_replica) { simple_lb_construct_replica(); } - } // namespace replication } // namespace dsn From 23076c63a8e2769eff6765526f0a587b909c39bd Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 31 Aug 2021 11:18:11 +0800 Subject: [PATCH 3/4] fix --- src/meta/test/meta_data.cpp | 76 ++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/src/meta/test/meta_data.cpp b/src/meta/test/meta_data.cpp index fa9b894944..a6ea6759b3 100644 --- a/src/meta/test/meta_data.cpp +++ b/src/meta/test/meta_data.cpp @@ -140,10 +140,10 @@ TEST(meta_data, collect_replica) // drop_list all have timestamp, full CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], 5, 1, 1, 2}, - dropped_replica{node_list[1], 6, 1, 1, 2}, - dropped_replica{node_list[2], 7, 1, 1, 2}, - dropped_replica{node_list[3], 8, 1, 1, 2}, + dropped_replica{node_list[0], 5, 1, 1, 2}, + dropped_replica{node_list[1], 6, 1, 1, 2}, + dropped_replica{node_list[2], 7, 1, 1, 2}, + dropped_replica{node_list[3], 8, 1, 1, 2}, }; rep.ballot = 10; rep.last_prepared_decree = 10; @@ -154,9 +154,9 @@ TEST(meta_data, collect_replica) // drop_list all have timestamp, not full CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], 5, 1, 1, 2}, - dropped_replica{node_list[1], 6, 1, 1, 2}, - dropped_replica{node_list[2], 7, 1, 1, 2}, + dropped_replica{node_list[0], 5, 1, 1, 2}, + dropped_replica{node_list[1], 6, 1, 1, 2}, + dropped_replica{node_list[2], 7, 1, 1, 2}, }; rep.ballot = 10; rep.last_durable_decree = 6; @@ -173,10 +173,10 @@ TEST(meta_data, collect_replica) // drop_list mixed, full, minimal position CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 3, 5}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 5}, - dropped_replica{node_list[2], 7, 1, 1, 5}, - dropped_replica{node_list[3], 8, 1, 1, 5}, + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 3, 5}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 5}, + dropped_replica{node_list[2], 7, 1, 1, 5}, + dropped_replica{node_list[3], 8, 1, 1, 5}, }; rep.ballot = 1; @@ -189,9 +189,9 @@ TEST(meta_data, collect_replica) // drop_list mixed, not full, minimal position CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 3, 5}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 5}, - dropped_replica{node_list[2], 7, 1, 1, 6}, + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 3, 5}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 5}, + dropped_replica{node_list[2], 7, 1, 1, 6}, }; rep.ballot = 1; @@ -208,10 +208,10 @@ TEST(meta_data, collect_replica) // drop_list mixed, full, not minimal position CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 6}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 6}, - dropped_replica{node_list[2], 7, 1, 1, 6}, - dropped_replica{node_list[3], 8, 1, 1, 6}, + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 6}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 6}, + dropped_replica{node_list[2], 7, 1, 1, 6}, + dropped_replica{node_list[3], 8, 1, 1, 6}, }; rep.ballot = 2; @@ -238,10 +238,10 @@ TEST(meta_data, collect_replica) ASSERT_TRUE(collect_replica(view, node_list[5], rep)); std::vector result_dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 6}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 6}, - dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 3, 1, 6}, - dropped_replica{node_list[2], 7, 1, 1, 6}}; + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 6}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 6}, + dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 3, 1, 6}, + dropped_replica{node_list[2], 7, 1, 1, 6}}; ASSERT_TRUE(vec_equal(result_dropped, cc.dropped)); } @@ -250,10 +250,10 @@ TEST(meta_data, collect_replica) // drop_list no timestamp, full, minimal position CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 8}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 8}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, }; rep.ballot = 1; @@ -266,10 +266,10 @@ TEST(meta_data, collect_replica) // drop_list no timestamp, full, middle position CLEAR_ALL; cc.dropped = { - dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 8}, - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, + dropped_replica{node_list[0], dropped_replica::INVALID_TIMESTAMP, 2, 2, 8}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, }; rep.ballot = 3; @@ -278,10 +278,10 @@ TEST(meta_data, collect_replica) ASSERT_TRUE(collect_replica(view, node_list[5], rep)); std::vector result_dropped = { - dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, - dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 3, 6, 8}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, + dropped_replica{node_list[1], dropped_replica::INVALID_TIMESTAMP, 2, 4, 8}, + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, + dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 3, 6, 8}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, }; ASSERT_TRUE(vec_equal(result_dropped, cc.dropped)); @@ -301,10 +301,10 @@ TEST(meta_data, collect_replica) ASSERT_TRUE(collect_replica(view, node_list[5], rep)); std::vector result_dropped = { - dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, - dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, - dropped_replica{node_list[4], dropped_replica::INVALID_TIMESTAMP, 4, 6, 8}, - dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 4, 8, 8}}; + dropped_replica{node_list[2], dropped_replica::INVALID_TIMESTAMP, 2, 6, 8}, + dropped_replica{node_list[3], dropped_replica::INVALID_TIMESTAMP, 4, 2, 8}, + dropped_replica{node_list[4], dropped_replica::INVALID_TIMESTAMP, 4, 6, 8}, + dropped_replica{node_list[5], dropped_replica::INVALID_TIMESTAMP, 4, 8, 8}}; ASSERT_TRUE(vec_equal(result_dropped, cc.dropped)); } From 18e321ab102de2b2789526a13ff5683be702faf2 Mon Sep 17 00:00:00 2001 From: levy Date: Tue, 31 Aug 2021 11:19:18 +0800 Subject: [PATCH 4/4] fix --- src/meta/meta_data.cpp | 123 +++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 61 deletions(-) diff --git a/src/meta/meta_data.cpp b/src/meta/meta_data.cpp index ba7e839ecf..1b92a38a4d 100644 --- a/src/meta/meta_data.cpp +++ b/src/meta/meta_data.cpp @@ -78,72 +78,73 @@ void maintain_drops(std::vector &drops, const rpc_address &node, co when_update_replicas(t, action); } -bool construct_replica(meta_view view, const gpid &pid, int max_replica_count) { - partition_configuration &pc = *get_config(*view.apps, pid); - config_context &cc = *get_config_context(*view.apps, pid); - - dassert(replica_count(pc) == 0, - "replica count of gpid(%d.%d) must be 0", - pid.get_app_id(), - pid.get_partition_index()); - dassert( - max_replica_count > 0, "max replica count is %d, should be at lease 1", max_replica_count); - - std::vector &drop_list = cc.dropped; - if (drop_list.empty()) { - dwarn("construct for (%d.%d) failed, coz no replicas collected", - pid.get_app_id(), - pid.get_partition_index()); - return false; - } +bool construct_replica(meta_view view, const gpid &pid, int max_replica_count) +{ + partition_configuration &pc = *get_config(*view.apps, pid); + config_context &cc = *get_config_context(*view.apps, pid); + + dassert(replica_count(pc) == 0, + "replica count of gpid(%d.%d) must be 0", + pid.get_app_id(), + pid.get_partition_index()); + dassert( + max_replica_count > 0, "max replica count is %d, should be at lease 1", max_replica_count); + + std::vector &drop_list = cc.dropped; + if (drop_list.empty()) { + dwarn("construct for (%d.%d) failed, coz no replicas collected", + pid.get_app_id(), + pid.get_partition_index()); + return false; + } - // treat last server in drop_list as the primary - const dropped_replica &server = drop_list.back(); - dassert(server.ballot != invalid_ballot, - "the ballot of server must not be invalid_ballot, node = %s", - server.node.to_string()); - pc.primary = server.node; - pc.ballot = server.ballot; - pc.partition_flags = 0; - pc.max_replica_count = max_replica_count; - - ddebug("construct for (%d.%d), select %s as primary, ballot(%" PRId64 - "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", + // treat last server in drop_list as the primary + const dropped_replica &server = drop_list.back(); + dassert(server.ballot != invalid_ballot, + "the ballot of server must not be invalid_ballot, node = %s", + server.node.to_string()); + pc.primary = server.node; + pc.ballot = server.ballot; + pc.partition_flags = 0; + pc.max_replica_count = max_replica_count; + + ddebug("construct for (%d.%d), select %s as primary, ballot(%" PRId64 + "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", + pid.get_app_id(), + pid.get_partition_index(), + server.node.to_string(), + server.ballot, + server.last_committed_decree, + server.last_prepared_decree); + + drop_list.pop_back(); + + // we put max_replica_count-1 recent replicas to last_drops, in case of the DDD-state when the + // only primary dead + // when add node to pc.last_drops, we don't remove it from our cc.drop_list + dassert(pc.last_drops.empty(), + "last_drops of partition(%d.%d) must be empty", + pid.get_app_id(), + pid.get_partition_index()); + for (auto iter = drop_list.rbegin(); iter != drop_list.rend(); ++iter) { + if (pc.last_drops.size() + 1 >= max_replica_count) + break; + // similar to cc.drop_list, pc.last_drop is also a stack structure + pc.last_drops.insert(pc.last_drops.begin(), iter->node); + ddebug("construct for (%d.%d), select %s into last_drops, ballot(%" PRId64 + "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", pid.get_app_id(), pid.get_partition_index(), - server.node.to_string(), - server.ballot, - server.last_committed_decree, - server.last_prepared_decree); - - drop_list.pop_back(); - - // we put max_replica_count-1 recent replicas to last_drops, in case of the DDD-state when the - // only primary dead - // when add node to pc.last_drops, we don't remove it from our cc.drop_list - dassert(pc.last_drops.empty(), - "last_drops of partition(%d.%d) must be empty", - pid.get_app_id(), - pid.get_partition_index()); - for (auto iter = drop_list.rbegin(); iter != drop_list.rend(); ++iter) { - if (pc.last_drops.size() + 1 >= max_replica_count) - break; - // similar to cc.drop_list, pc.last_drop is also a stack structure - pc.last_drops.insert(pc.last_drops.begin(), iter->node); - ddebug("construct for (%d.%d), select %s into last_drops, ballot(%" PRId64 - "), committed_decree(%" PRId64 "), prepare_decree(%" PRId64 ")", - pid.get_app_id(), - pid.get_partition_index(), - iter->node.to_string(), - iter->ballot, - iter->last_committed_decree, - iter->last_prepared_decree); - } - - cc.prefered_dropped = (int) drop_list.size() - 1; - return true; + iter->node.to_string(), + iter->ballot, + iter->last_committed_decree, + iter->last_prepared_decree); } + cc.prefered_dropped = (int)drop_list.size() - 1; + return true; +} + bool collect_replica(meta_view view, const rpc_address &node, const replica_info &info) { partition_configuration &pc = *get_config(*view.apps, info.pid);