Samunroyu commented on code in PR #2328:
URL:
https://github.com/apache/incubator-pegasus/pull/2328#discussion_r3418284815
##########
src/meta/meta_split_service.cpp:
##########
@@ -311,10 +311,12 @@ void
meta_split_service::on_add_child_on_remote_storage_reply(error_code ec,
// TODO(yingchun): should use conference?
auto child_pc = app->pcs[child_gpid.get_partition_index()];
child_pc.secondaries = request.child_config.secondaries;
- child_pc.__set_hp_secondaries(request.child_config.hp_secondaries);
+ std::vector<host_port> secondaries;
+ GET_HOST_PORTS(request.child_config, secondaries, secondaries);
Review Comment:
> `child_pc.secondaries` (rpc_address list) is copied from
`request.child_config.secondaries`, while `child_pc.hp_secondaries` is derived
via `GET_HOST_PORTS(...)` (which may source from either `secondaries` or
`hp_secondaries`). If `request.child_config` only has one representation
populated, `child_pc.secondaries` and `child_pc.hp_secondaries` can become
inconsistent. To avoid mixed-source state, set both fields from the same source
(e.g., derive rpc addresses from the computed `secondaries` host_port list, or
use a helper/macro that sets both representations together).
>
> Suggested change
> GET_HOST_PORTS(request.child_config, secondaries, secondaries);
> GET_HOST_PORTS(child_pc, secondaries, secondaries);
The current implementation is correct. GET_HOST_PORTS handles backward
compatibility: if request.child_config.hp_secondaries is not set (old version),
it derives from request.child_config.secondaries. Both child_pc.secondaries and
child_pc.hp_secondaries ultimately come from the same source. Using
SET_OBJ_IP_AND_HOST_PORT would fail
when hp_secondaries is not populated in the source.
--
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]
