Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(update_replication_factor#12): update the mutation_2pc_min_replica_count base the max count of table #1035

Merged
merged 15 commits into from
Jul 12, 2022

Conversation

foreverneverer
Copy link
Contributor

@foreverneverer foreverneverer commented Jul 6, 2022

Ref-Issue

#865

What problem does this PR solve?

#865 support different replica factor, but mutation_2pc_min_replica_count still keep 2, if one table update the factor to 2, when one replica is down(for example, offline node case), the table would keep write reject status.

So mutation_2pc_min_replica_count should be updated base the table factor, this pr should set it replica_max_count /2 + 1, but consider that sometimes user need set the replica_max_count > 3, I think the config mutation_2pc_min_replica_count is still valid:

  1. if user set the mutation_2pc_min_replica_count > 0, the final 2pc_min_replica_count = mutation_2pc_min_replica_count
  2. otherwise, 2pc_min_replica_count = app_max_replica_count <= 2 ? 1 : app_max_replica_count / 2 + 1 which can be make sure the table is alive though more node crush

Config-Change

if you want use value of config.ini, you don't need update it, otherwise:

- mutation_2pc_min_replica_count = 2
+ mutation_2pc_min_replica_count = 0

What is changed and how does it work?

Checklist

Tests
  • Unit test
  • Manual test (add detailed scripts or steps below)
1. start server in test cluster
4. create table with 3 replica count
5. start read/set with 10k qps
6. set replica count = 2
7. kill one node
8. watch the write status
Related changes
  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to be included in the release note

@foreverneverer foreverneverer changed the title fix fix: update the mutation_2pc_min_replica_count base the max count of table Jul 7, 2022
@foreverneverer foreverneverer changed the title fix: update the mutation_2pc_min_replica_count base the max count of table fix(update_replication_factor#12): update the mutation_2pc_min_replica_count base the max count of table Jul 8, 2022
@foreverneverer foreverneverer marked this pull request as ready for review July 8, 2022 03:22
rdsn/src/replica/replica.h Outdated Show resolved Hide resolved
rdsn/src/meta/meta_service.h Outdated Show resolved Hide resolved
rdsn/src/common/replication_common.h Outdated Show resolved Hide resolved
rdsn/src/common/replication_common.h Outdated Show resolved Hide resolved
rdsn/src/common/replication_common.h Outdated Show resolved Hide resolved
rdsn/src/common/replication_common.h Outdated Show resolved Hide resolved
rdsn/src/meta/meta_service.cpp Outdated Show resolved Hide resolved
rdsn/src/meta/meta_service.cpp Outdated Show resolved Hide resolved
empiredan
empiredan previously approved these changes Jul 12, 2022
@acelyc111 acelyc111 merged commit 502fd20 into apache:master Jul 12, 2022
@foreverneverer foreverneverer mentioned this pull request Jul 13, 2022
ZhongChaoqiang pushed a commit to ZhongChaoqiang/incubator-pegasus that referenced this pull request Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants