-
Notifications
You must be signed in to change notification settings - Fork 469
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
Expand production recommendations #2850
Conversation
@bdarnell and @a-robinson, can either or both of you review this and help me get it to a decent state prior to the 2.0 release? Sorry for the short notice. |
d743989
to
706e277
Compare
706e277
to
d1d6a55
Compare
Reviewed 2 of 6 files at r1, 3 of 3 files at r2. v2.0/recommended-production-settings.md, line 34 at r1 (raw file):
I wouldn't recommend setting the v2.0/recommended-production-settings.md, line 40 at r1 (raw file):
This should probably be bulked up with explanations of how you need three datacenters to be able to survive the failure of one. (a common misconception, as in this forum thread today). v2.0/recommended-production-settings.md, line 46 at r1 (raw file):
What does this example have to do with "using different numbers and types of nodes per region"? I'm not seeing anything actionable here and would probably just remove this paragraph and diagram. Comments from Reviewable |
Reviewed 2 of 6 files at r1, 3 of 3 files at r2. v2.0/recommended-production-settings.md, line 24 at r2 (raw file):
We probably also want to mention here that CockroachDB spreads the replicas of each piece of data across as diverse a set of localities as possible (unless configured to do otherwise via replication zones) v2.0/recommended-production-settings.md, line 43 at r2 (raw file):
Follow the workload is great and all, but the biggest reason to use localities when you have multiple datacenters is to ensure each piece of data ends up with one copy in each datacenter. If you don't set localities, we don't differentiate between the nodes and will almost certainly end up with 2 out of 3 copies for some important ranges in just one datacenter, meaning that losing the datacenter would cause an outage. v2.0/recommended-production-settings.md, line 44 at r2 (raw file):
Minor suggestion, feel free to ignore: I'd reword this to "To optimize read and write latency for specific subsets of rows within a table, ..." Comments from Reviewable |
- Update cluster topology to mention locality and cover common cluster patterns. Addresses part of #2411. - Add a security section. - Add locality to manual deployment tutorials.
I simplified the cluster topology changes, trying to take into account the feedback provided. PTAL, Ben and Alex. Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed. v2.0/recommended-production-settings.md, line 40 at r1 (raw file): Previously, bdarnell (Ben Darnell) wrote…
Tried to address this. PTAL. v2.0/recommended-production-settings.md, line 46 at r1 (raw file): Previously, bdarnell (Ben Darnell) wrote…
OK, removed. I was trying to follow @rober-s-lee's guidance in #2411, but it seems best to review that with you or another engineer before attempting to document it. v2.0/recommended-production-settings.md, line 24 at r2 (raw file): Previously, a-robinson (Alex Robinson) wrote…
Done. PTAL. v2.0/recommended-production-settings.md, line 43 at r2 (raw file): Previously, a-robinson (Alex Robinson) wrote…
Tried to address this. PTAL. v2.0/recommended-production-settings.md, line 44 at r2 (raw file): Previously, a-robinson (Alex Robinson) wrote…
Done. Comments from Reviewable |
d1d6a55
to
35a7586
Compare
Review status: 3 of 4 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed. Comments from Reviewable |
Review status: 3 of 4 files reviewed at latest revision, 6 unresolved discussions, some commit checks failed. Comments from Reviewable |
TFTRs, @bdarnell and @a-robinson. Did some final tweaking, but I don't think it's necessary to review again. |
cluster patterns. Addresses part of Basic topology patterns #2411.