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

docs: Fix rounding for determining max number of failing zones #6896

Closed
wants to merge 2 commits into from

Conversation

aknuds1
Copy link
Contributor

@aknuds1 aknuds1 commented Dec 12, 2023

What this PR does

In docs, fix formula for determining max number of failing zones. The current phrasing combined with downwards rounding (floor) means you may end up requiring too few failing zones. E.g., if RF is 3, we would require fewer than 1 failing zone, while it should be max 1.

Examples:

  • replication factor 3, max 1 (floor(3 / 2)) failing zone
  • replication factor 6, max 3 (floor(6/2)) failing zones
  • replication factor 9, max 4 (floor(9/2)) failing zones

Which issue(s) this PR fixes or relates to

Checklist

  • [na] Tests updated.
  • Documentation added.
  • [na] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [na] about-versioning.md updated with experimental features.

@aknuds1 aknuds1 requested review from a team as code owners December 12, 2023 10:45
@aknuds1 aknuds1 added type/docs Improvements or additions to documentation bug Something isn't working labels Dec 12, 2023
@@ -69,7 +69,7 @@ With a replication factor of 3, which is the default, deploy the Grafana Mimir c
Deploying Grafana Mimir clusters to more zones than the configured replication factor does not have a negative impact.
Deploying Grafana Mimir clusters to fewer zones than the configured replication factor can cause writes to the replica to be missed, or can cause writes to fail completely.

If there are fewer than `floor(replication factor / 2)` zones with failing replicas, reads and writes can withstand zone failures.
If there are fewer than `ceil(replication factor / 2)` zones with failing replicas, reads and writes can withstand zone failures.
Copy link
Contributor

Choose a reason for hiding this comment

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

This entire phrasing is confusing and backwards to me. Instead of saying "fewer than X zones with failing replicas" can we say "There can be at most X zones with failing replicas otherwise reads and writes will fail"?

Copy link
Contributor Author

@aknuds1 aknuds1 Dec 12, 2023

Choose a reason for hiding this comment

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

Agree w/ your sentiment @56quarters. I'll try to revise into something more straightforward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewrote it according to your suggestion, PTAL.

@aknuds1 aknuds1 force-pushed the arve/docs-failing-replicas branch from 478f8f4 to cd3891d Compare January 16, 2024 10:09
@aknuds1 aknuds1 requested a review from 56quarters January 16, 2024 10:09
@@ -69,7 +69,7 @@ With a replication factor of 3, which is the default, deploy the Grafana Mimir c
Deploying Grafana Mimir clusters to more zones than the configured replication factor does not have a negative impact.
Deploying Grafana Mimir clusters to fewer zones than the configured replication factor can cause writes to the replica to be missed, or can cause writes to fail completely.

If there are fewer than `floor(replication factor / 2)` zones with failing replicas, reads and writes can withstand zone failures.
There can be at most `floor(replication factor / 2)` zones with failing replicas, otherwise reads and writes will fail.
Copy link
Contributor

@56quarters 56quarters Jan 16, 2024

Choose a reason for hiding this comment

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

I think this formula is still wrong. We need a majority of zones available to accept reads and writes.

With a replication factor = 3, this formula gives us floor(3 / 2) = 1 one zone that can fail -- that's correct. However, with replication factor = 4, this formula gives us floor(4 / 2) = 2 two zones that can fail -- that's not correct. We need 3 zones with a replication factor of 4 (even though we don't recommend even replication factors). With replication factor = 5, this formula gives us floor(5 / 2) = 2 two zones that can fail -- correct.

I believe the correct formula is "there can be at most floor((replication factor - 1) / 2) zones with failing replicas".

Repeating the above scenario:

With a replication factor = 3, this formula gives us floor((3 - 1) / 2) = 1 one zone that can fail -- that's correct. With replication factor = 4, this formula gives us floor((4 - 1) / 2) = 1 one zone that can fail -- still correct. With replication factor = 5, this formula gives us floor((5 - 1) / 2) = 2 two zones that can fail -- still correct.

Please double check my reasoning and math.

Copy link
Member

Choose a reason for hiding this comment

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

I've always thought of Mimir failure tolerance in terms of quorum because I first encountered this operating etcd. I remembered their docs being pretty good about this aspect so I dug them out:

https://etcd.io/docs/v3.3/faq/#why-an-odd-number-of-cluster-members:

An etcd cluster needs a majority of nodes, a quorum, to agree on updates to the cluster state.
For a cluster with n members, quorum is (n/2)+1.

https://etcd.io/docs/v3.3/faq/#what-is-failure-tolerance

Cluster Size Majority Failure Tolerance
1 1 0
2 2 0
3 2 1
4 3 1
5 3 2
6 4 2
7 4 3
8 5 3
9 5 4

I think the table is a really handy way of covering both sides (quorum and failure tolerance) and is easy to read for those less comfortable with the formula.

@aknuds1 aknuds1 force-pushed the arve/docs-failing-replicas branch from cd3891d to b825793 Compare March 5, 2024 07:31
@aknuds1 aknuds1 requested a review from jdbaldry as a code owner March 5, 2024 07:31
@aknuds1 aknuds1 force-pushed the arve/docs-failing-replicas branch from b825793 to aed7de7 Compare June 19, 2024 15:42
@56quarters
Copy link
Contributor

Superseded by #9512

@56quarters 56quarters closed this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working type/docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants