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

Configuring replication zones #189

Merged
merged 11 commits into from
Apr 3, 2016
Merged

Conversation

jseldess
Copy link
Contributor

@jseldess jseldess commented Apr 1, 2016

First draft of docs on configuring replication zones. Want to know what I've missed or got wrong, obviously, but also ideas for how to make this content clearer or easier to digest. Also welcome ideas for more helpful or realistic examples.

@petermattis, @bdarnell, @spencerkimball

Pushed content to aws in case html version is easier to read: http://cockroach-draft-docs.s3-website-us-east-1.amazonaws.com/configure-replication-zones.html


This change is Reviewable

@petermattis
Copy link
Contributor

Review status: 0 of 4 files reviewed at latest revision, 6 unresolved discussions.


configure-replication-zones.md, line 30 [r1] (raw file):
s/it uses a replication/it uses the replication/g? I'm not sure about this, but I think when we're talking about the replication zone for a table or database the grammar should be the replication zone. Feel free to correct me about this if I'm wrong.


configure-replication-zones.md, line 34 [r1] (raw file):
s/is in/is specified in/g


configure-replication-zones.md, line 46 [r1] (raw file):
Rather than say "randomly choose", I think this should read: the replica can be placed on any node in the cluster.

We should probably also mention that the number of replicas should be odd as otherwise we can encounter situations where both sides of a network partition can form a quorum. In fact, we should just disallow even number of replicas. Do you mind filing a bug about that?

s/no specific attributes/no attributes/g


configure-replication-zones.md, line 49 [r1] (raw file):
s/a zone/the zone/g

ttlseconds is implemented, but I don't think we should document it yet as we have no means of accessing older versions.


configure-replication-zones.md, line 161 [r1] (raw file):
I think you should use single quote shell strings (i.e. ') instead of double quote to avoid any possibility of weird expansion.

No need to specify --insecure any more.


configure-replication-zones.md, line 239 [r1] (raw file):
Perhaps zone set should echo the full zone config that was set so that the user doesn't need to zone get to see it.


Comments from the review on Reviewable.io

@jseldess
Copy link
Contributor Author

jseldess commented Apr 1, 2016

Review status: 0 of 4 files reviewed at latest revision, 6 unresolved discussions.


configure-replication-zones.md, line 30 [r1] (raw file):
Yep, fixing with next commit.


configure-replication-zones.md, line 34 [r1] (raw file):
Fixing with next commit.


configure-replication-zones.md, line 46 [r1] (raw file):
Ok, changing this with next commit. I think "the replica can be placed on any node" is a little ambiguous in terms of who is taking that action (cockroach or user), but not a big deal.

Filed bug to disallow even number of replicas in a zone: cockroachdb/cockroach#5809


configure-replication-zones.md, line 49 [r1] (raw file):
OK, can you explain what ttlseconds is again? Would like to provide something basic.


configure-replication-zones.md, line 161 [r1] (raw file):
Fixing throughout with next commit.

In terms of --insecure, though, these examples are implicitly multi-machine, so you would need --insecure, right?


configure-replication-zones.md, line 239 [r1] (raw file):
Done with next commit.


Comments from the review on Reviewable.io

@petermattis
Copy link
Contributor

:lgtm:


Review status: 0 of 4 files reviewed at latest revision, 4 unresolved discussions.


configure-replication-zones.md, line 49 [r1] (raw file):
We store multiple versions of a value for each key in our low-level KV storage. ttlseconds controls how long a version will be kept around after it has been replaced by a newer version. The default setting keeps old versions around for 1 day before deleting them.


configure-replication-zones.md, line 161 [r1] (raw file):
Ah, excellent point. I retract my comment.


configure-replication-zones.md, line 239 [r1] (raw file):
Well, this will need to be done in the code. Can you file an issue?


Comments from the review on Reviewable.io

@jseldess
Copy link
Contributor Author

jseldess commented Apr 1, 2016

Review status: 0 of 4 files reviewed at latest revision, 5 unresolved discussions.


configure-replication-zones.md, line 30 [r1] (raw file):
Done.


configure-replication-zones.md, line 49 [r1] (raw file):
Thanks. I'll say something vague for now.


configure-replication-zones.md, line 239 [r1] (raw file):
Don't understand. What needs to be done in the code?


Comments from the review on Reviewable.io

@petermattis
Copy link
Contributor

Review status: 0 of 4 files reviewed at latest revision, 4 unresolved discussions.


configure-replication-zones.md, line 46 [r1] (raw file):
Replicas are always assigned to nodes by cockroach. While a user can provide guidance, the actual assignment is up to the system.


configure-replication-zones.md, line 239 [r1] (raw file):
The zone set` command should echo the full zone config that it set.


Comments from the review on Reviewable.io

@jseldess
Copy link
Contributor Author

jseldess commented Apr 2, 2016

@bdarnell, @petermattis, please check the new Node/Replica Recommendations section.

@bdarnell
Copy link
Contributor

bdarnell commented Apr 2, 2016

LGTM


Review status: 0 of 5 files reviewed at latest revision, 7 unresolved discussions.


configure-replication-zones.md, line 46 [r5] (raw file):
We should emphasize that even though repeating attrs: [] looks weird, it's perfectly normal to not have any attributes and it's common that the only thing you'll do with a zone config is change the number of attrs lines. People shouldn't feel pressured to "fill in the blanks" with attrs if they don't need them.

Also be aware that we're planning on reworking this interface so you can specify a number of replicas independent of attributes: cockroachdb/cockroach#4868


configure-replication-zones.md, line 47 [r5] (raw file):
Express the default as 64MB.


configure-replication-zones.md, line 57 [r5] (raw file):
The previous sentence is good, but I don't like this one as much. I think it would be better to explain why even numbers are bad: "Configurations with odd numbers of replicas are more robust than those with even numbers. Clusters of three and four nodes can each tolerate one node failure and still reach a quorum (2/3 and 3/4 respectively), so the fourth replica doesn't add any extra fault-tolerance. To survive two simultaneous failures, you must have five replicas."


configure-replication-zones.md, line 197 [r5] (raw file):
Attributes should be something that are shared across many machines. With this configuration if one of the nodes went down, the data would be underreplicated until a new node was given the attribute of the downed one. Instead, the attribute should be ssd in all cases; the system knows that it needs to find three different nodes with the attribute so it won't put all the replicas in one place.


Comments from Reviewable

@jseldess
Copy link
Contributor Author

jseldess commented Apr 3, 2016

Review status: 0 of 5 files reviewed at latest revision, 6 unresolved discussions.


configure-replication-zones.md, line 239 [r1] (raw file):
Created an issue for this: cockroachdb/cockroach#5819


configure-replication-zones.md, line 46 [r5] (raw file):
I'll work on improving that. Thanks, Ben.


configure-replication-zones.md, line 47 [r5] (raw file):
Will do, Ben. But does this mean that the field accepts any bytes-based unit, with the suffix required for anything but straight bytes?


configure-replication-zones.md, line 57 [r5] (raw file):
This is excellent, Ben. Thanks!


configure-replication-zones.md, line 197 [r5] (raw file):
Makes sense. Fixing with next commit.


Comments from Reviewable

@jseldess jseldess merged commit 31342d7 into gh-pages Apr 3, 2016
@jseldess jseldess deleted the jseldess/zone-configuration branch April 3, 2016 03:39
@bdarnell
Copy link
Contributor

bdarnell commented Apr 3, 2016

Review status: 0 of 6 files reviewed at latest revision, 6 unresolved discussions.


configure-replication-zones.md, line 47 [r5] (raw file):
I don't think it does, but it probably should.


Comments from Reviewable

@jseldess
Copy link
Contributor Author

jseldess commented Apr 3, 2016

configure-replication-zones.md, line 47 [r5] (raw file):
Opened an issue for this: cockroachdb/cockroach#5828


Comments from Reviewable

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