acelyc111 commented on code in PR #1608:
URL:
https://github.com/apache/incubator-pegasus/pull/1608#discussion_r1470642456
##########
src/replica/duplication/duplication_sync_timer.h:
##########
@@ -73,6 +73,10 @@ class duplication_sync_timer
std::vector<replica_ptr> get_all_replicas();
+ // duplication_sync_timer is async with replica close,so
replica_duplicator_manager may already
+ // release dup should stop right now
Review Comment:
Just describe what does this function do is OK, you can describe why invoke
it when in the caller.
##########
src/replica/duplication/duplication_sync_timer.cpp:
##########
@@ -211,5 +232,14 @@ duplication_sync_timer::get_dup_states(int app_id, /*out*/
bool *app_found)
return result;
}
+bool duplication_sync_timer::replica_is_cloing_or_closed(gpid id)
+{
+ if (_stub->_closing_replicas.find(id) != _stub->_closing_replicas.end() ||
Review Comment:
I think it's possible to judge the replica's state by itself, maybe by
`status()`, it's not needed to access replica_stub.
##########
src/replica/duplication/duplication_sync_timer.cpp:
##########
@@ -113,12 +117,28 @@ void duplication_sync_timer::update_duplication_map(
{
for (replica_ptr &r : get_all_replicas()) {
auto it = dup_map.find(r->get_gpid().get_app_id());
+ app_info *info = const_cast<app_info *>(r->get_app_info());
+ gpid id = r->get_gpid();
+ bool doing_duplication;
+ if (replica_is_cloing_or_closed(id)) {
+ continue;
+ }
+
if (it == dup_map.end()) {
// no duplication is assigned to this app
r->get_duplication_manager()->update_duplication_map({});
+ doing_duplication = false;
} else {
r->get_duplication_manager()->update_duplication_map(it->second);
+ doing_duplication = true;
+ }
+
+ if (doing_duplication) {
+ info->__set_duplicating(true);
Review Comment:
It would be better to update the info in replica class to avoid concurrent
write, see replica::update_app_max_replica_count() for example.
##########
src/replica/duplication/duplication_sync_timer.cpp:
##########
@@ -211,5 +232,14 @@ duplication_sync_timer::get_dup_states(int app_id, /*out*/
bool *app_found)
return result;
}
+bool duplication_sync_timer::replica_is_cloing_or_closed(gpid id)
+{
+ if (_stub->_closing_replicas.find(id) != _stub->_closing_replicas.end() ||
Review Comment:
Move this function to replica_stub is more meaningful, and it's needed to
protected by `_replicas_lock` to avoid data race.
--
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]
