Skip to content
This repository has been archived by the owner on Jun 23, 2022. It is now read-only.

feat(split): small refactor and fix of replica split #636

Merged
merged 5 commits into from
Oct 12, 2020

Conversation

hycdong
Copy link
Contributor

@hycdong hycdong commented Oct 10, 2020

This pull request is about replica_split_manager, including:

  • update some log and comments
  • update rpc call

@hycdong hycdong changed the title refactor: small refactor of replica split feat(split): small refactor and fix of replica split Oct 10, 2020
@hycdong hycdong marked this pull request as ready for review October 10, 2020 03:24
Comment on lines 788 to 793
if (status() != partition_status::PS_PARTITION_SPLIT) {
dwarn_replica("child partition has been active, status={}", enum_to_string(status()));
return;
}

ddebug_replica("child partition become active");
Copy link
Contributor

@neverchanje neverchanje Oct 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    if (status() != partition_status::PS_PARTITION_SPLIT) {
        dwarn_replica("child partition has been active, status={}", enum_to_string(status()));
        return;
    }

    ddebug_replica("child partition become active");

The two logs seem ambiguous. They both say the partition is "active". I think you should add a comment here, stating what it means "active". And I guess the latter log should be "is becoming active".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I move the latter log after child status changed, it will be clearer.

@hycdong hycdong merged commit d063842 into XiaoMi:master Oct 12, 2020
@hycdong hycdong deleted the refactor_replica_split branch October 12, 2020 10:18
hycdong added a commit to hycdong/rdsn that referenced this pull request Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants