empiredan commented on code in PR #1960:
URL:
https://github.com/apache/incubator-pegasus/pull/1960#discussion_r1541033951
##########
src/meta/duplication/meta_duplication_service.cpp:
##########
@@ -158,76 +184,125 @@ void
meta_duplication_service::add_duplication(duplication_add_rpc rpc)
const auto &request = rpc.request();
auto &response = rpc.response();
- LOG_INFO("add duplication for app({}), remote cluster name is {}",
+ std::string remote_app_name;
+ if (request.__isset.remote_app_name) {
+ remote_app_name = request.remote_app_name;
+ } else {
+ // Once remote_app_name is not specified by client, use source
app_name as
+ // remote_app_name to be compatible with old versions(< v2.6.0) of
client.
+ remote_app_name = request.app_name;
+ }
+
+ LOG_INFO("add duplication for app({}), remote cluster name is {}"
+ "remote app name is {}",
request.app_name,
- request.remote_cluster_name);
+ request.remote_cluster_name,
+ remote_app_name);
- response.err = ERR_OK;
+ LOG_WARNING_DUP_HINT_AND_RETURN_IF_NOT(request.remote_cluster_name !=
+ get_current_cluster_name(),
+ response,
+ "illegal operation: adding
duplication to itself");
- if (request.remote_cluster_name == get_current_cluster_name()) {
- response.err = ERR_INVALID_PARAMETERS;
- response.__set_hint("illegal operation: adding duplication to itself");
- return;
- }
auto remote_cluster_id =
get_duplication_cluster_id(request.remote_cluster_name);
- if (!remote_cluster_id.is_ok()) {
- response.err = ERR_INVALID_PARAMETERS;
- response.__set_hint(fmt::format("get_duplication_cluster_id({})
failed, error: {}",
- request.remote_cluster_name,
- remote_cluster_id.get_error()));
- return;
- }
+ LOG_WARNING_DUP_HINT_AND_RETURN_IF_NOT(remote_cluster_id.is_ok(),
+ response,
+ "get_duplication_cluster_id({})
failed, error: {}",
+ request.remote_cluster_name,
+ remote_cluster_id.get_error());
std::vector<host_port> meta_list;
- if (!dsn::replication::replica_helper::load_meta_servers(
- meta_list,
- duplication_constants::kClustersSectionName.c_str(),
- request.remote_cluster_name.c_str())) {
- response.err = ERR_INVALID_PARAMETERS;
- response.__set_hint(fmt::format("failed to find cluster[{}] address in
config [{}]",
- request.remote_cluster_name,
-
duplication_constants::kClustersSectionName));
- return;
- }
-
- auto app = _state->get_app(request.app_name);
- if (!app || app->status != app_status::AS_AVAILABLE) {
- response.err = ERR_APP_NOT_EXIST;
- return;
- }
+
LOG_WARNING_DUP_HINT_AND_RETURN_IF_NOT(dsn::replication::replica_helper::load_meta_servers(
+ meta_list,
+
duplication_constants::kClustersSectionName.c_str(),
+
request.remote_cluster_name.c_str()),
+ response,
+ "failed to find cluster[{}] address
in config [{}]",
+ request.remote_cluster_name,
+
duplication_constants::kClustersSectionName);
+
+ std::shared_ptr<app_state> app;
duplication_info_s_ptr dup;
- for (const auto &ent : app->duplications) {
- auto it = ent.second;
- if (it->follower_cluster_name == request.remote_cluster_name) {
- dup = ent.second;
- break;
+ {
+ zauto_read_lock l(app_lock());
+
+ app = _state->get_app(request.app_name);
+ if (!app || app->status != app_status::AS_AVAILABLE) {
+ response.err = ERR_APP_NOT_EXIST;
+ LOG_WARNING("app {} was not found", request.app_name);
+ return;
+ }
+
+ for (const auto &ent : app->duplications) {
+ if (ent.second->remote_cluster_name ==
request.remote_cluster_name) {
+ dup = ent.second;
+ break;
+ }
+ }
+
+ if (dup) {
+ // The duplication for the same app to the same remote cluster has
existed.
+ remote_app_name = dup->remote_app_name;
+ LOG_INFO("no need to add duplication, since it has existed:
app_name={}, "
+ "remote_cluster_name={}, remote_app_name={}",
+ request.app_name,
+ request.remote_cluster_name,
+ remote_app_name);
+ } else {
+ // Check if other apps of this cluster are duplicated to the same
remote app.
+ for (const auto & [ app_name, cur_app_state ] :
_state->_exist_apps) {
+ if (app_name == request.app_name) {
+ // Skip this app since we want to check other apps.
+ continue;
+ }
+
+ for (const auto &ent : cur_app_state->duplications) {
+ LOG_WARNING_DUP_HINT_AND_RETURN_IF_NOT(
+ ent.second->remote_cluster_name !=
request.remote_cluster_name ||
Review Comment:
Actually the invalid case is that `dup_info->remote_cluster_name ==
request.remote_cluster_name && dup_info->remote_app_name == remote_app_name`.
--
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]
