Skip to content
This repository was archived by the owner on Oct 17, 2022. It is now read-only.

Suggested edit to make clear what a quorum actually represents.#252

Merged
flimzy merged 5 commits into
apache:masterfrom
oliverdain:master
Mar 8, 2018
Merged

Suggested edit to make clear what a quorum actually represents.#252
flimzy merged 5 commits into
apache:masterfrom
oliverdain:master

Conversation

@oliverdain

Copy link
Copy Markdown
Contributor

Overview

I found the documentation about read and write quorums for a cluster mode DB confusing. I think this edit makes things a bit more clear (though I don't love the wording).

This came from a Slack conversation which you can find here if curious: https://couchdb.slack.com/archives/C49LEE7NW/p1519761842000347

@flimzy flimzy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I applaud the attempt at clarity. Unfortunately, the edit isn't very clear to me--the first paragraph in particular.

Comment thread src/cluster/theory.rst Outdated
save a document before a write is returned with ``201``. If there are less
nodes than that number, then ``202`` is returned. Both read and write numbers
can be specified with a request as ``r`` and ``w`` parameters accordingly.
the document plus one *in the current partition*. That is, if there is a network

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"partition" is not a word common to CouchDB parlance. Do you mean node? shard?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was trying to describe a network partition. Specifically, here I'm trying to say that if there's a network partition then the quorum is relative to only the subset of nodes on the side of the partition where the client is.

Definitely open to different wording if this is confusing.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, now I see what you're getting at. That is an important point. And the paragraph was already a bit confusing before your addition. Perhaps a rewrite of the entire section is in order.

Example:

In clustered operation, a quorum must be reached before CouchDB returns a 200 for a fetch, or 201 for a write operation. A quorum is defined one plus half the number of copies.

For read operations, the maximum number of copies is the number of currently-accessible nodes, meaning that in the case of a failure or network partition, the effective number of copies may be lower. The number of read copies can be set with the rparameter.

For write operations, the number of copies can be set using the w parameter. If fewer than this number of nodes is available, a 202 will be returned.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Overall I like what you have better. Will start with that but make a few changes to be more explicit. Updated PR coming soon...

Comment thread src/cluster/theory.rst Outdated
The number of nodes that need to save a document before a write is returned with
``201`` is also equal to half of the total copies of the document plus one.
However, unlike the read case, for writes this is computed for the database as a
whole, even if there is a network partition. If there are less nodes than that

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"less" -> "fewer" to satisfy the grammar nazis

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will fix.

@oliverdain

Copy link
Copy Markdown
Contributor Author

Thanks for the review @flimzy . Made the obvious grammar fix. Not entirely sure how to fix the 1st paragraph. Would love suggestions.

@oliverdain

Copy link
Copy Markdown
Contributor Author

OK. Ready for re-review. Thanks @flimzy

@flimzy flimzy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I like this wording much better! Well done. Just a few minor fixes remaining.

Comment thread src/cluster/theory.rst Outdated
nodes than that number, then ``202`` is returned. Both read and write numbers
can be specified with a request as ``r`` and ``w`` parameters accordingly.
In clustered operation, a quorum must be reached before CouchDB returns a 200
for a fetch, or 201 for a write operation. A quorum is defined one plus half the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A quorum is defined as one...

Comment thread src/cluster/theory.rst Outdated
save a document before a write is returned with ``201``. If there are less
nodes than that number, then ``202`` is returned. Both read and write numbers
can be specified with a request as ``r`` and ``w`` parameters accordingly.
In clustered operation, a quorum must be reached before CouchDB returns a 200

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Probably good idea to wrap status codes with tick marks as before:

``200``

vs

200

Comment thread src/cluster/theory.rst Outdated
set with the rparameter.

For write operations the number of relevant copies is always `n`, the number of
replicas in teh cluster. For write operations, the number of copies can be set

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

teh -> the

@oliverdain

Copy link
Copy Markdown
Contributor Author

All fixed. Thanks for the review.

@flimzy

flimzy commented Mar 5, 2018

Copy link
Copy Markdown
Member

+1

@oliverdain

Copy link
Copy Markdown
Contributor Author

@flimzy what's your process? Do you do the merge or should I do that?

Thanks again for the review.

@flimzy
flimzy merged commit 4eabf79 into apache:master Mar 8, 2018
@oliverdain

Copy link
Copy Markdown
Contributor Author

Hey @flimzy thanks again for your review here. I noticed the live docs haven't changed yet (http://docs.couchdb.org/en/2.1.1/cluster/theory.html). I kinda thought when this was merged the docs would auto-update. Is that not the case?

@wohali

wohali commented Mar 15, 2018

Copy link
Copy Markdown
Member

Hi @oliverdain ,

The documentation for a specific release (such as 2.1.1) are invariant; they are tagged at the time of the official Apache CouchDB release and updates cannot be made.

You can see your updates here: http://docs.couchdb.org/en/master/cluster/theory.html

In addition, the next release of CouchDB (2.2.0) will include your changes. This release is Very Soon Now(tm).

Thanks again for your contribution!

rnewson added a commit that referenced this pull request Dec 15, 2020
…s. (#252)"

This reverts commit 4eabf79.

This notion of different quorums for doc read vs doc write is not true.
rnewson added a commit that referenced this pull request Dec 15, 2020
…s. (#252)" (#610)

This reverts commit 4eabf79.

This notion of different quorums for doc read vs doc write is not true.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants