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

Make writable partition selection account for partitions with more than 3 replicas #1349

Merged
merged 5 commits into from
Jan 23, 2020

Conversation

jsjtzyy
Copy link
Contributor

@jsjtzyy jsjtzyy commented Jan 3, 2020

Currently, selecting writable partition for PUT operation only picks partitions with highest local replica count. This will become a problem when "move replica" is rolled out. In the intermediate state of "move replica", particular partition may have 6 replicas and if we stick with current logic. All the write traffic will be routed to this particular partition. Hence, this PR we make changes to allow partition selection to pick partitions with replicas count >= min replica count(specified in
ClusterMapConfig). In most cases, min replica count = 3.

…e than 3 replicas

Currently, selecting writable partiton for PUT operation only picks
partitions with highest local replica count. This will become a problem
when "move replica" is rolled out. In the intermediate state of "move
replica", particular partion may have 6 replicas and if we stick with
current logic. All the write traffic will be routed to this particular
partition. Hence, this PR we make changes to allow partition selection
to pick partitons with replicas count >= min replica count(specified in
ClusterMapConfig). In most cases, min replica count = 3.
@jsjtzyy jsjtzyy self-assigned this Jan 3, 2020
@codecov-io
Copy link

codecov-io commented Jan 4, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@817325a). Click here to learn what that means.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1349   +/-   ##
=========================================
  Coverage          ?   71.97%           
  Complexity        ?     6673           
=========================================
  Files             ?      485           
  Lines             ?    38031           
  Branches          ?     4820           
=========================================
  Hits              ?    27373           
  Misses            ?     9357           
  Partials          ?     1301
Impacted Files Coverage Δ Complexity Δ
...ub.ambry.clustermap/HelixBootstrapUpgradeUtil.java 84.81% <0%> (ø) 110 <0> (?)
.../com.github.ambry/clustermap/PartitionManager.java 0% <0%> (ø) 0 <0> (?)
...m.github.ambry.clustermap/HelixClusterManager.java 87.57% <100%> (ø) 57 <3> (?)
...a/com.github.ambry.clustermap/PartitionLayout.java 69.92% <100%> (ø) 34 <0> (?)
...b.ambry.clustermap/StaticClusterAgentsFactory.java 32.43% <100%> (ø) 4 <0> (?)
...java/com.github.ambry/config/ClusterMapConfig.java 100% <100%> (ø) 3 <0> (?)
...a/com.github.ambry.clustermap/ClusterMapUtils.java 93.78% <77.77%> (ø) 44 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 817325a...f1ac05f. Read the comment docs.

@jsjtzyy jsjtzyy changed the title WIP: Make writable partition selection account for partitions with more than 3 replicas Make writable partition selection account for partitions with more than 3 replicas Jan 4, 2020
@jsjtzyy jsjtzyy marked this pull request as ready for review January 4, 2020 18:17
@jsjtzyy jsjtzyy requested a review from lightningrob January 22, 2020 21:57
Copy link
Contributor

@lightningrob lightningrob left a comment

Choose a reason for hiding this comment

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

LGTM. Please reformat files before we merge.

@@ -261,5 +269,6 @@ public ClusterMapConfig(VerifiableProperties verifiableProperties) {
verifiableProperties.getLongInRange("clustermap.replica.catchup.acceptable.lag.bytes", 0L, 0L, Long.MAX_VALUE);
clustermapReplicaCatchupTarget =
verifiableProperties.getIntInRange("clustermap.replica.catchup.target", 0, 0, Integer.MAX_VALUE);
clustermapWritablePartitionMinReplicaCount = verifiableProperties.getIntInRange("clustermap.writable.partition.min.replica.count", 3, 0, Integer.MAX_VALUE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: might need formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I am on it

@lightningrob lightningrob merged commit fcdfca8 into linkedin:master Jan 23, 2020
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