empiredan commented on code in PR #2217:
URL:
https://github.com/apache/incubator-pegasus/pull/2217#discussion_r2081337455
##########
src/meta/duplication/meta_duplication_service.cpp:
##########
@@ -764,21 +764,24 @@ void
meta_duplication_service::check_follower_app_if_create_completed(
query_err = ERR_INCONSISTENT_STATE;
} else {
for (const auto &pc : resp.partitions) {
- if (!pc.hp_primary) {
+ host_port primary;
+ std::vector<host_port> secondaries;
+ GET_HOST_PORT(pc, primary, primary);
+ GET_HOST_PORTS(pc, secondaries, secondaries);
+ if (!primary) {
// Fail once the primary replica is unavailable.
query_err = ERR_INACTIVE_STATE;
break;
}
// Once replica count is more than 1, at least one
secondary replica
// is required.
- if (1 + pc.hp_secondaries.size() <
pc.max_replica_count &&
- pc.hp_secondaries.empty()) {
+ if (1 + secondaries.size() < pc.max_replica_count &&
secondaries.empty()) {
Review Comment:
```suggestion
std::vector<host_port> secondaries;
GET_HOST_PORTS(pc, secondaries, secondaries);
if (1 + secondaries.size() < pc.max_replica_count &&
secondaries.empty()) {
```
##########
src/meta/duplication/meta_duplication_service.cpp:
##########
@@ -764,21 +764,24 @@ void
meta_duplication_service::check_follower_app_if_create_completed(
query_err = ERR_INCONSISTENT_STATE;
} else {
for (const auto &pc : resp.partitions) {
- if (!pc.hp_primary) {
+ host_port primary;
+ std::vector<host_port> secondaries;
+ GET_HOST_PORT(pc, primary, primary);
+ GET_HOST_PORTS(pc, secondaries, secondaries);
+ if (!primary) {
Review Comment:
```suggestion
host_port primary;
GET_HOST_PORT(pc, primary, primary);
if (!primary) {
```
##########
src/replica/replica_context.cpp:
##########
@@ -133,11 +133,17 @@ void
primary_context::get_replica_config(partition_status::type st,
bool primary_context::check_exist(const ::dsn::host_port &node,
partition_status::type st)
{
+ host_port primary;
+ std::vector<host_port> secondaries;
+ GET_HOST_PORT(pc, primary, primary);
+ GET_HOST_PORTS(pc, secondaries, secondaries);
switch (st) {
case partition_status::PS_PRIMARY:
- return pc.hp_primary == node;
+ DCHECK(pc.__isset.hp_primary, "");
+ return primary == node;
Review Comment:
```suggestion
{
DCHECK(pc.__isset.hp_primary, "");
host_port primary;
GET_HOST_PORT(pc, primary, primary);
return primary == node;
}
```
##########
src/meta/greedy_load_balancer.cpp:
##########
@@ -147,7 +147,13 @@ bool
greedy_load_balancer::all_replica_infos_collected(const node_state &ns)
{
const auto &n = ns.host_port();
return ns.for_each_partition([this, n](const dsn::gpid &pid) {
- config_context &cc = *get_config_context(*(t_global_view->apps), pid);
+ config_context *ctx = get_config_context(*(t_global_view->apps), pid);
+ if (ctx == nullptr) {
+ LOG_INFO("get_config_context return nullptr for gpid({})", pid);
+ return false;
+ }
+
+ config_context &cc = *ctx;
if (cc.find_from_serving(n) == cc.serving.end()) {
Review Comment:
```suggestion
if (ctx->find_from_serving(n) == ctx->serving.end()) {
```
##########
src/replica/replica_context.cpp:
##########
@@ -133,11 +133,17 @@ void
primary_context::get_replica_config(partition_status::type st,
bool primary_context::check_exist(const ::dsn::host_port &node,
partition_status::type st)
{
+ host_port primary;
+ std::vector<host_port> secondaries;
+ GET_HOST_PORT(pc, primary, primary);
+ GET_HOST_PORTS(pc, secondaries, secondaries);
switch (st) {
case partition_status::PS_PRIMARY:
- return pc.hp_primary == node;
+ DCHECK(pc.__isset.hp_primary, "");
+ return primary == node;
case partition_status::PS_SECONDARY:
- return utils::contains(pc.hp_secondaries, node);
+ DCHECK(pc.__isset.hp_secondaries, "");
+ return utils::contains(secondaries, node);
Review Comment:
```suggestion
{
DCHECK(pc.__isset.hp_secondaries, "");
std::vector<host_port> secondaries;
GET_HOST_PORTS(pc, secondaries, secondaries);
return utils::contains(secondaries, node);
}
```
##########
src/meta/server_state_restore.cpp:
##########
@@ -251,7 +251,11 @@ void
server_state::on_query_restore_status(configuration_query_restore_rpc rpc)
for (int32_t i = 0; i < app->partition_count; i++) {
const auto &r_state = app->helpers->restore_states[i];
const auto &pc = app->pcs[i];
- if (pc.hp_primary || !pc.hp_secondaries.empty()) {
+ host_port primary;
+ std::vector<host_port> secondaries;
+ GET_HOST_PORT(pc, primary, primary);
+ GET_HOST_PORTS(pc, secondaries, secondaries);
+ if (primary || !secondaries.empty()) {
Review Comment:
```suggestion
host_port primary;
GET_HOST_PORT(pc, primary, primary);
if (primary) {
// Since the primary already exists, the restore succeeds.
continue;
}
std::vector<host_port> secondaries;
GET_HOST_PORTS(pc, secondaries, secondaries);
if (!secondaries.empty()) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
