Add global and regional tables to patterns docs #10465
Conversation
|
This change is |
|
Files changed: _includes/sidebar-data-v21.1.json |
|
Netlify Preview Built with commit f89ebe4 |
There was a problem hiding this comment.
Reviewed 38 of 38 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @rmloveland)
a discussion (no related file):
Very well done! LGTM
_includes/sidebar-data-v21.1.json, line 682 at r1 (raw file):
"title": "Global Tables", "urls": [ "/${VERSION}/topology-global-tables.html"
Can we remove topology here or is that just unnecessary extra work?
_includes/v21.1/sql/global-table-description.md, line 5 at r1 (raw file):
Use global tables when your application has a "read-mostly" table of reference data that is rarely updated, and needs to be available to all regions. For an example of a table that can benefit from the _global_ table locality setting in a multi-region deployment, see the `promo_codes` table from the [MovR application](movr.html).
Maybe a line like other candidates include a postal codes table etc?
_includes/v21.1/sql/regional-by-row-table-description.md, line 1 at r1 (raw file):
In _regional by row_ tables, individual rows are optimized for access from different regions. This setting divides a table and all of [its indexes](multiregion-overview.html#indexes-on-regional-by-row-tables) into [partitions](partitioning.html), with each partition optimized for access from a different region. Like [regional tables](multiregion-overview.html#regional-tables), _regional by row_ tables are optimized for access from a single region. However, that region is specified at the row level instead of applying to the whole table.
This setting "automatically" divides
_includes/v21.1/sql/regional-table-description.md, line 3 at r1 (raw file):
Regional tables work well when your application requires low-latency reads and writes for an entire table from a single region. For _regional_ tables, access to the table will be fast in the table's "home region" and slower in other regions. In other words, CockroachDB optimizes access to data in regional tables from a single region. By default, a regional table's home region is the [database's primary region](multiregion-overview.html#database-regions), but that can be changed to use any region added to the database.
how about "any region in the database"
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r1 (raw file):
Most users should not need to use partitioning directly. Instead, they should use CockroachDB's built-in [multi-region capabilities](multiregion-overview.html), which automatically handle geo-partitioning and other low-level details.
Nit Handles
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 4 at r1 (raw file):
- What are my [survival goals](multiregion-overview.html#survival-goals)? Do I need to survive a [zone failure](multiregion-overview.html#surviving-zone-failures)? A [region failure](multiregion-overview.html#surviving-region-failures)? - Given the constraints provided by my survival goals, what are the [table localities](multiregion-overview.html#table-locality) that will provide the performance characteristics I need for each table's data?
I dont think you actually need to think about this directly. You can think of your survival goals and then think of your latency goals but you are correct that they interact. My question is, is it worth to explain that they do? I think not.
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
- What are my [survival goals](multiregion-overview.html#survival-goals)? Do I need to survive a [zone failure](multiregion-overview.html#surviving-zone-failures)? A [region failure](multiregion-overview.html#surviving-region-failures)? - Given the constraints provided by my survival goals, what are the [table localities](multiregion-overview.html#table-locality) that will provide the performance characteristics I need for each table's data? - Do I need low-latency reads and writes from a single region? Do I need them at the [row level](multiregion-overview.html#regional-by-row-tables)? Or will the [table level](multiregion-overview.html#regional-tables) suffice?
I'd flip table and row here so it shows most likely answer first
v21.1/alter-primary-key.md, line 236 at r1 (raw file):
CONFIGURE ZONE USING constraints = '[+region=us-east1]'; ~~~
so. many. deletes. Awesome! 👏
v21.1/computed-columns.md, line 17 at r1 (raw file):
- **Secondary indexes** can be created on computed columns, which is especially useful when a table is frequently sorted. See the [secondary indexes example](#create-a-table-with-a-secondary-index-on-a-computed-column) below. - **Partitioning** requires that partitions are defined using columns that are a prefix of the [primary key](primary-key.html). In the case of geo-partitioning, some applications will want to collapse the number of possible values in this column, to make certain classes of queries more performant. For example, if a users table has a country and state column, then you can make a stored computed column locality with a reduced domain for use in partitioning.
Should we even keep partitioning here? I'd suggest deleting
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
- Finally, make the `postal_codes` table a [global table](multiregion-overview.html#global-tables). ~~~
Why did we decide to show the zone configurations here?
v21.1/topology-regional-tables.md, line 82 at r1 (raw file):
ALTER TABLE users ALTER COLUMN region SET NOT NULL; ALTER TABLE users SET LOCALITY REGIONAL BY ROW AS "region"; ~~~
I think you should discuss that we could have set it automatically but we are choosing to define the column here because of x y and z
v21.1/topology-regional-tables.md, line 89 at r1 (raw file):
- Put the leaseholders in `us-east`, since it's the [primary database region](multiregion-overview.html#database-regions). - Make the `users` table a [regional by row table](#regional-by-row-tables) by [partitioning](partitioning.html) the [primary key index](primary-key.html) by region. When rows are added or updated, which region the row is associated is specified as part of the update. For details, see the instructions for [updating a row's home region](set-locality.html#crdb_region). Thanks to CockroachDB's [multi-region capabilities](multiregion-overview.html), you do not need to do any partitioning "by hand", the database does it for you based on your desired [table locality setting](multiregion-overview.html#table-locality).
same zone configs questions
rmloveland
force-pushed
the
20210422-multiregion-capabilities
branch
from
e702ee9
to
4754570
Compare
|
Files changed: _includes/sidebar-data-v21.1.json |
There was a problem hiding this comment.
Reviewable status:
a discussion (no related file):
Previously, awoods187 (Andy Woods) wrote…
Very well done! LGTM
Thanks for the review Andy! I just pushed some changes based on your feedback
_includes/sidebar-data-v21.1.json, line 682 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
Can we remove topology here or is that just unnecessary extra work?
Removed it altogether.
Also as you probably noted already I've tried to remove "topology" from most of the inline content too
_includes/v21.1/sql/global-table-description.md, line 5 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
Maybe a line like other candidates include a postal codes table etc?
we had an extra postal_code e.g. line and removed it based on comment by @ajstorm in #9723 (review) (search for "maybe it's just me")
This is the same text that was vetted in that PR, but broken out into an "include fragment" to be re-used in multiple places
_includes/v21.1/sql/regional-by-row-table-description.md, line 1 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
This setting "automatically" divides
Done
_includes/v21.1/sql/regional-table-description.md, line 3 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
how about "any region in the database"
Works for me! Updated
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
Most users should not need to use partitioning directly. Instead, they should use CockroachDB's built-in [multi-region capabilities](multiregion-overview.html), which automatically handle geo-partitioning and other low-level details.Nit Handles
Nice, thanks. Edited this one a few times and the errors, they sneak in
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 4 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
I dont think you actually need to think about this directly. You can think of your survival goals and then think of your latency goals but you are correct that they interact. My question is, is it worth to explain that they do? I think not.
Do you mean delete the clause "given the constraints provided by my survival goals" from second bullet?
If yes, happy to.
If no, what is the "this" the user does not need to think about?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
I'd flip table and row here so it shows most likely answer first
Done, reversed the order
v21.1/alter-primary-key.md, line 236 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
so. many. deletes. Awesome! 👏
yayyyyy
thanks Andy!
v21.1/computed-columns.md, line 17 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
Should we even keep partitioning here? I'd suggest deleting
OK I'll delete it altogether, let's commit to this :-)
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
Why did we decide to show the zone configurations here?
tl;dr: tradeoffs. Was def torn on this.
My intention at the start of this cycle was to avoid zone configs entirely. However we are not really 100% there yet (see also: non-voting replicas), plus the difficulty of squishing something about metrics into this page that doesn't have a real cluster running associated with it, so this is a tradeoff.
Basically right after showing the configs, the page says "PS a better way to check this is via metrics on a running cluster, shown in this other tutorial". Showing the zone configs here is kind of a nod to the fact that this page is more "tell" than "show". Since this is not associated with a real running cluster, showing zone configs is a quicker way to say "here's how you can verify this happened" - since I can not in fact show you in this context a metrics chart that is accurate to anything.
Also tbh checking the metrics chart on a cluster still takes a lot of setup and a looooooong wall clock time watching the metrics waiting for them to go down -- relative to glancing at zone configs to see if "the setting took", which is basically an instantaneous activity
Edits to these "patterns" pages in the v21.1 cycle are also part of the tradeoff. In 1-2 releases I would like to replace them altogether, but they collectively take up a lot of "room" in the docs (and are linked from/to so many many many many things). I felt ripping them out would create a bad "who moved my cheese" information situation, since up until now they have been very prominent. As you can see from the size of this PR they are very woven in.
v21.1/topology-regional-tables.md, line 82 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
I think you should discuss that we could have set it automatically but we are choosing to define the column here because of x y and z
Added a note that says (basically) "TIMTOWTDI: there's more than one way to do it - see also this other doc, which has all the docs on how to set the region column, etc."
v21.1/topology-regional-tables.md, line 89 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
same zone configs questions
See reply above
There was a problem hiding this comment.
Reviewed 35 of 38 files at r1, 13 of 13 files at r2.
Reviewable status:
_includes/sidebar-data-v21.1.json, line 659 at r1 (raw file):
}, { "title": "Topology Patterns",
Do we want to keep this section about "Topology Patterns" split off from the previous one about "Multi-Region Capabilities"?
_includes/sidebar-data-v21.1.json, line 680 at r1 (raw file):
}, { "title": "Global Tables",
Do we need to reconsider whether these pages fall under "Develop" or "Deploy", now that table locality is a schema attribute?
_includes/sidebar-data-v21.1.json, line 680 at r1 (raw file):
}, { "title": "Global Tables",
Should we order REGIONAL tables first? That's the order we have them listed everywhere else, and it's the order in which we introduce the concepts.
_includes/v21.1/sql/global-table-description.md, line 5 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
we had an extra
postal_codee.g. line and removed it based on comment by @ajstorm in #9723 (review) (search for "maybe it's just me")This is the same text that was vetted in that PR, but broken out into an "include fragment" to be re-used in multiple places
Another candidate SGTM, though I also don't think it's needed and would avid postal codes, since they are by their very nature localized, which may introduce confusion. Can we find something better?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 1 at r1 (raw file):
Multi-region patterns require thinking about the following questions:
Do we link to this page from anywhere?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
- What are my [survival goals](multiregion-overview.html#survival-goals)? Do I need to survive a [zone failure](multiregion-overview.html#surviving-zone-failures)? A [region failure](multiregion-overview.html#surviving-region-failures)? - Given the constraints provided by my survival goals, what are the [table localities](multiregion-overview.html#table-locality) that will provide the performance characteristics I need for each table's data? - Do I need low-latency reads and writes from a single region? Do I need them at the [row level](multiregion-overview.html#regional-by-row-tables)? Or will the [table level](multiregion-overview.html#regional-tables) suffice?
"low latency reads and writes at the row levels" isn't actually the question, right? What even is a "read at the row level"? I think what we're trying to say is something along the lines of "Do I need that single region to be configurable at the row level?"
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 6 at r1 (raw file):
- Given the constraints provided by my survival goals, what are the [table localities](multiregion-overview.html#table-locality) that will provide the performance characteristics I need for each table's data? - Do I need low-latency reads and writes from a single region? Do I need them at the [row level](multiregion-overview.html#regional-by-row-tables)? Or will the [table level](multiregion-overview.html#regional-tables) suffice? - Do I have a "read-mostly" [table of reference data that is rarely updated](multiregion-overview.html#global-tables), but that needs to be available to all regions?
"Available" might imply that REGIONAL tables are not available in non-primary regions. But that's not true. They're still available, they're just not optimal to access from a latency perspective.
v21.1/cost-based-optimizer.md, line 196 at r1 (raw file):
- Hint usage should be reconsidered with each new release of CockroachDB. Due to improvements in the optimizer, hints specified to work with an older version may cause decreased performance in a newer version. ## Preferring the nearest index
Do we need this section at all anymore? Isn't it entirely obviated by GLOBAL tables?
v21.1/topology-follow-the-workload.md, line 15 at r1 (raw file):
{{site.data.alerts.callout_success}} If read performance is your main focus for a table, but you want low-latency reads everywhere instead of just in the most active region, consider the [Global Table Locality Pattern](topology-global-tables.html) or [Follower Reads](topology-follower-reads.html) pattern.
nit: We're using the work "pattern" twice here.
v21.1/topology-follower-reads.md, line 15 at r1 (raw file):
{{site.data.alerts.callout_success}} If reads from a table must be exactly up-to-date, use the [Global Table Locality Pattern](topology-global-tables.html) or [Regional Table Locality Pattern](topology-regional-tables.html) instead. Up-to-date reads are required by tables referenced by [foreign keys](foreign-key.html), for example.
Do we want to call these "patterns"? I thought we had gotten away from that.
v21.1/global-tables.md, line 107 at r2 (raw file):
## Alternatives - If your application can tolerate historical reads in some cases, consider the [Follower Reads](topology-follower-reads.html) pattern.
It's a little unclear from reading this how Follower Reads relate to REGIONAL tables. I think we're going to want to clarify that. In this new world, stale follower reads are mostly a property of REGIONAL tables, because there's little reason to use them on GLOBAL tables.
v21.1/regional-tables.md, line 47 at r2 (raw file):
{% include {{page.version.version}}/sql/regional-by-row-table-description.md %} ### Steps
Should we include steps for REGIONAL tables as well? We seem to jump right to REGIONAL BY ROW tables.
v21.1/regional-tables.md, line 68 at r2 (raw file):
{% include copy-clipboard.html %} ~~~ sql ALTER TABLE users ADD COLUMN region crdb_internal_region AS (
I don't think we want to show the "derived" REGIONAL BY ROW AS form of REGIONAL BY ROW tables as the first example. This is really an extension that skips over a lot of what makes REGIONAL BY ROW tables special. It requires the user to manually create a column instead of allowing CRDB to create it and it only works if that table already has a form of locality tracked in one of its columns.
As a result, we also skip over a lot of the auto-homing and locality optimized search stuff that is crucial for standard REGIONAL BY ROW tables but not as important when the crdb_region column is computed.
There was a problem hiding this comment.
Reviewed 2 of 38 files at r1.
Reviewable status:
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r1 (raw file):
{{site.data.alerts.callout_success}} <span class="version-tag">New in v21.1:</span> Most users should not need to use partitioning directly. Instead, they should use CockroachDB's built-in [multi-region capabilities](multiregion-overview.html), which automatically handle geo-partitioning and other low-level details.
Nit: I'm wondering if we want to give a bit more justification here. Like "Most users should not need to use partitioning directly, as the primary reason for partitioning tables in CockroachDB is to assign partitions to specific geographical regions." (or something like that)
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 4 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Do you mean delete the clause "given the constraints provided by my survival goals" from second bullet?
If yes, happy to.
If no, what is the "this" the user does not need to think about?
Yes, I think that's what he's saying.
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 6 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
"Available" might imply that REGIONAL tables are not available in non-primary regions. But that's not true. They're still available, they're just not optimal to access from a latency perspective.
"...that must be read with low latency from all regions"?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 17 at r2 (raw file):
In addition, reviewing the following information will be helpful: - The concept of [locality](cockroach-start.html#locality), which makes CockroachDB aware of the location of nodes and able to intelligently place and balance data based on how you define [survival goals](multiregion-overview.html#survival-goals) and [table localities](multiregion-overview.html#table-locality).
Nit: We might want to drop the part after "...aware of the location of nodes". Alternatively, it might be easier to read this if there was only one link per bullet.
_includes/v21.1/topology-patterns/see-also.md, line 4 at r2 (raw file):
- [Choosing a multi-region configuration](choosing-a-multi-region-configuration.html) - [When to use `ZONE` vs. `REGION` survival goals](when-to-use-zone-vs-region-survival-goals.html) - [When to use `GLOBAL` vs. `REGIONAL` tables](when-to-use-regional-vs-global-tables.html)
Nit: order is reversed in link.
_includes/v21.1/topology-patterns/see-also.md, line 14 at r2 (raw file):
- [`REGIONAL` Table Locality Pattern](regional-tables.html) - [`GLOBAL` Table Locality Pattern](global-tables.html) - [Follow-the-Workload](topology-follow-the-workload.html)
Do we want to say something about deprecating FTW?
v21.1/cockroach-demo.md, line 58 at r2 (raw file):
~~~ shell $ cockroach demo --geo-partitioned-replicas <other flags>
I thought that we wanted to de-emphasize --geo-partitioned-replicas. Are we thinking that we can't fully do that until we have the changes made to demo that @awoods187 is working on?
v21.1/disaster-recovery.md, line 164 at r2 (raw file):
{{site.data.alerts.callout_success}} <span class="version-tag">New in v21.1:</span> By default, every [multi-region database](multiregion-overview.html) has a [survival goal](multiregion-overview.html#survival-goals) associated with it. The survival goal setting provides an abstraction that handles the low-level details of replica placement to ensure your desired fault tolerance. The information below is still useful for legacy deployments.
Nit: Do we want to say here what that default setting is (zone survivability)?
v21.1/topology-patterns.md, line 34 at r2 (raw file):
{{site.data.alerts.callout_info}} The multi-region patterns described below are almost always table-specific. For example, you might use [Regional Tables](regional-tables.html) for frequently updated tables that are geographically specific, and [Global Tables](global-tables.html) pattern for reference tables that are not tied to geography and that are read frequently but updated infrequently.
Nit: "not tied to geography" -> "not tied to a specific region"?
v21.1/topology-patterns.md, line 34 at r2 (raw file):
{{site.data.alerts.callout_info}} The multi-region patterns described below are almost always table-specific. For example, you might use [Regional Tables](regional-tables.html) for frequently updated tables that are geographically specific, and [Global Tables](global-tables.html) pattern for reference tables that are not tied to geography and that are read frequently but updated infrequently.
Nit: "that are geographically specific" -> "that are region specific"?
v21.1/topology-patterns.md, line 37 at r2 (raw file):
{{site.data.alerts.end}} | Pattern | Latency | Resiliency |
Nit: not sure if I commented on this earlier, but it's a bit strange to me that we have the Resiliency column which is the same for every row. Can we have a Resiliency note at the bottom of this table which refers to the fact that Resiliency is set at the database level?
v21.1/topology-patterns.md, line 40 at r2 (raw file):
|----------------------------------------------------------+------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------| | [Regional Tables](regional-tables.html) | Low latency for single-region writes and multi-region stale reads. | Depends on your [survival goals](multiregion-overview.html#survival-goals). | | [Global Tables](global-tables.html) | Low-latency multi-region reads; writes are higher latency than reads. | Depends on your [survival goals](multiregion-overview.html#survival-goals). |
At the risk of being a bit pedantic, writes will always be higher latency than reads, even for a single node database. Can we frame this instead as "Low-latency multi-region reads from all regions, at the expense of higher latency (cross-region) writes"?
v21.1/topology-patterns.md, line 42 at r2 (raw file):
| [Global Tables](global-tables.html) | Low-latency multi-region reads; writes are higher latency than reads. | Depends on your [survival goals](multiregion-overview.html#survival-goals). | | [Follower Reads](topology-follower-reads.html) | Fast regional (historical) reads, slower cross-region writes. | Depends on your [survival goals](multiregion-overview.html#survival-goals). | | [Follow-the-Workload](topology-follow-the-workload.html) | Fast regional reads in the active region; slower cross-region reads elsewhere. Slower cross-region writes. | Depends on your [survival goals](multiregion-overview.html#survival-goals). |
FTW is not compatible with the new syntax (I believe), so the survival goal entry in this row is incorrect.
v21.1/global-tables.md, line 12 at r2 (raw file):
- Read latency must be low, but write latency can be much higher. - Reads must be up-to-date for business reasons or because the table is referenced by [foreign keys](foreign-key.html). - Rows in the table, and all latency-sensitive queries, **cannot** be tied to specific geographies.
Nit: specific regions?
Nit: Do we want to say "latency-sensitive reads"? Some people think of writes as SQL queries, even though they shouldn't.
v21.1/global-tables.md, line 38 at r2 (raw file):
### Summary Using this pattern, you tell CockroachDB to set the [table locality](multiregion-overview.html#table-locality) to `GLOBAL`, and CockroachDB handles the details.
Nit: this part strikes me as a bit strange, as we talk about the details, but never explain what those details are.
v21.1/regional-tables.md, line 13 at r2 (raw file):
- Read and write latency must be low. - Rows in the table, and all latency-sensitive queries, can be tied to specific geographies, e.g., city, state, region.
Nit: Might just be me, but I'm a bit worried that users might find the use of "geographies" confusing, as we only expose the concept of "regions". Is it possible to change it to "geographical region" and drop the "e.g."?
v21.1/regional-tables.md, line 64 at r2 (raw file):
~~~ Since all tables in a multi-region cluster default to the [`REGIONAL BY TABLE`](#regional-tables) locality setting, let's set the table's locality to [`REGIONAL BY ROW`](#regional-by-row-tables) using the following [`ALTER TABLE`](alter-table.html) statements: [`ADD COLUMN`](add-column.html), [`ALTER COLUMN`](alter-column.html), and [`SET LOCALITY`](set-locality.html).
Nit: We could show that the table defaulted to REGIONAL BY TABLE by running SHOW CREATE TABLE here.
v21.1/regional-tables.md, line 68 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
I don't think we want to show the "derived"
REGIONAL BY ROW ASform ofREGIONAL BY ROWtables as the first example. This is really an extension that skips over a lot of what makes REGIONAL BY ROW tables special. It requires the user to manually create a column instead of allowing CRDB to create it and it only works if that table already has a form of locality tracked in one of its columns.As a result, we also skip over a lot of the auto-homing and locality optimized search stuff that is crucial for standard REGIONAL BY ROW tables but not as important when the
crdb_regioncolumn is computed.
+1
v21.1/regional-tables.md, line 88 at r2 (raw file):
{{site.data.alerts.end}} To confirm that your `users` table data is replicated across the cluster in accordance with the `REGIONAL BY ROW` table locality, check the **Data Distribution** [debug page](ui-debug-pages.html) in the [DB Console](ui-overview.html). It will look something like the output below (which is edited for length). Translating from zone configurations into human language, this output says:
I find this part a bit odd, because we're explaining the zone config, but in a way that may only make sense to someone who already understands zone configurations (and in some detail). Also, we're not really explaining that the zone configuration is applied at each partition. My vote would be to remove this zone config description. To the point above about customers worrying that we moved their cheese, I'm not too concerned about that, since the hiding is the main point of this multi-region work.
v21.1/regional-tables.md, line 92 at r2 (raw file):
- Make the database resilient to zone failures, with replicas in each region (this is the default [`ZONE` survival goal](multiregion-overview.html#survival-goals)). - Put the leaseholders in `us-east`, since it's the [primary database region](multiregion-overview.html#database-regions). - Make the `users` table a [regional by row table](#regional-by-row-tables) by [partitioning](partitioning.html) the [primary key index](primary-key.html) by region. When rows are added or updated, which region the row is associated is specified as part of the update. For details, see the instructions for [updating a row's home region](set-locality.html#crdb_region). Thanks to CockroachDB's [multi-region capabilities](multiregion-overview.html), you do not need to do any partitioning "by hand", the database does it for you based on your desired [table locality setting](multiregion-overview.html#table-locality).
"When rows are added or updated, which region the row is associated is specified as part of the update." - is this missing a "to"? Perhaps "When rows are added or updated, the region to which the row is associated is specified as part of the update."
v21.1/regional-tables.md, line 124 at r2 (raw file):
### Latency For [`REGIONAL BY TABLE` tables](#regional-tables), you get low latency for single-region writes and multi-region stale reads.
For single region writes and reads?
v21.1/regional-tables.md, line 126 at r2 (raw file):
For [`REGIONAL BY TABLE` tables](#regional-tables), you get low latency for single-region writes and multi-region stale reads. For [`REGIONAL BY ROW` tables](#regional-by-row-tables), you get low-latency consistent multi-region reads & writes for rows which are homed in specific regions.
...and low-latency multi-region stale reads from all other regions?
v21.1/regional-tables.md, line 130 at r2 (raw file):
### Resiliency Because the `test` database does not specify a [survival goal](multiregion-overview.html#survival-goals), it uses the default [`ZONE` survival goal](multiregion-overview.html#surviving-zone-failures). With the default settings, an entire AZ can fail without interrupting access to the database.
Nit: we might want to spell out AZ here.
v21.1/regional-tables.md, line 136 at r2 (raw file):
## Alternatives - If rows in the table **cannot** be tied to specific geographies, and reads must be up-to-date for business reasons or because the table is referenced by [foreign keys](foreign-key.html), consider the [`GLOBAL` Table Locality Pattern](global-tables.html).
...and the table is rarely modified?
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
tl;dr: tradeoffs. Was def torn on this.
My intention at the start of this cycle was to avoid zone configs entirely. However we are not really 100% there yet (see also: non-voting replicas), plus the difficulty of squishing something about metrics into this page that doesn't have a real cluster running associated with it, so this is a tradeoff.
Basically right after showing the configs, the page says "PS a better way to check this is via metrics on a running cluster, shown in this other tutorial". Showing the zone configs here is kind of a nod to the fact that this page is more "tell" than "show". Since this is not associated with a real running cluster, showing zone configs is a quicker way to say "here's how you can verify this happened" - since I can not in fact show you in this context a metrics chart that is accurate to anything.
Also tbh checking the metrics chart on a cluster still takes a lot of setup and a looooooong wall clock time watching the metrics waiting for them to go down -- relative to glancing at zone configs to see if "the setting took", which is basically an instantaneous activity
Edits to these "patterns" pages in the v21.1 cycle are also part of the tradeoff. In 1-2 releases I would like to replace them altogether, but they collectively take up a lot of "room" in the docs (and are linked from/to so many many many many things). I felt ripping them out would create a bad "who moved my cheese" information situation, since up until now they have been very prominent. As you can see from the size of this PR they are very woven in.
Not suggesting we do this in the current PR, but could we not get something similar with SHOW CREATE TABLE?
v21.1/topology-regional-tables.md, line 82 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Added a note that says (basically) "TIMTOWTDI: there's more than one way to do it - see also this other doc, which has all the docs on how to set the region column, etc."
Does it make sense to start with the standard RBR pattern, and have the TIMTOWTDI refer to a computed column example?
There was a problem hiding this comment.
Reviewable status:
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 4 at r1 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Yes, I think that's what he's saying.
yes exactly
v21.1/cockroach-demo.md, line 58 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
I thought that we wanted to de-emphasize
--geo-partitioned-replicas. Are we thinking that we can't fully do that until we have the changes made to demo that @awoods187 is working on?
I think rich suggested we not delete it since someone might still want to see it as a comparison. Im fine deleting or keeping.
v21.1/topology-follower-reads.md, line 15 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we want to call these "patterns"? I thought we had gotten away from that.
+1
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
showing zone configs is a quicker way to say "here's how you can verify this happened"
Yeah but then you have to explain both zone configs and how to read show zone configs which is very confusing and hard. I think we should do show create table. The contract should be like any other SQL, you see the SQL, you don't have to see what that translates into. I'd vote deleting but can disagree and committ.
rmloveland
force-pushed
the
20210422-multiregion-capabilities
branch
from
4754570
to
3beaca4
Compare
|
Files changed: _includes/sidebar-data-v21.1.json |
There was a problem hiding this comment.
TFTR Andy, Adam, and Nathan!
I think almost (?) all of the comments have been addressed. PTAL!
Reviewable status:
_includes/sidebar-data-v21.1.json, line 659 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we want to keep this section about "Topology Patterns" split off from the previous one about "Multi-Region Capabilities"?
This is a good question. There's a lot of overlap.
After some playing around I moved it under "Multi-Region Capabilities" as a subsection, since 5/6 of the pages are about "multi-region things" anyway
Let me know what you think. The "topology pattern" term is mostly gone from this section of the docs now, except in the name of that one overview page.
_includes/sidebar-data-v21.1.json, line 680 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we need to reconsider whether these pages fall under "Develop" or "Deploy", now that table locality is a schema attribute?
I think yes
But I think that will have to be part of a larger doc project to overhaul our TOC tree in general. The whole "topology pattern" thing was not really about topologies to begin with (since it mentions follower reads etc), so I think our docs TOC could use a conceptual clarification / overhaul.
_includes/sidebar-data-v21.1.json, line 680 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Should we order REGIONAL tables first? That's the order we have them listed everywhere else, and it's the order in which we introduce the concepts.
Good call. They are ordered first now in the new organization (described in previous comment)
_includes/v21.1/sql/global-table-description.md, line 5 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Another candidate SGTM, though I also don't think it's needed and would avid postal codes, since they are by their very nature localized, which may introduce confusion. Can we find something better?
We probably can find something better, but I'd rather punt on this for now since good examples are hard and we already have at least this one with a "real" schema
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r1 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: I'm wondering if we want to give a bit more justification here. Like "Most users should not need to use partitioning directly, as the primary reason for partitioning tables in CockroachDB is to assign partitions to specific geographical regions." (or something like that)
I'd rather not go more into detail about partitioning (other than to say it exists). My thesis is that if you need to know about partitioning because you have a need that is not met by the multi-region abstractions, you are probably an expert doing something "interesting" and you can def go read the multiple pages of docs we already have on the subject, and if you don't you can ignore it. We do have a link to the partitioning docs from the multi-region overview page, FWIW
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 1 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we link to this page from anywhere?
This is an "included fragment" that shows up on the "regional tables" and "global tables" pages.
$ ack 'topology-patterns\/multiregion-fundamentals'
regional-tables.md
23:{% include {{ page.version.version }}/topology-patterns/multiregion-fundamentals.md %}
global-tables.md
24:{% include {{ page.version.version }}/topology-patterns/multiregion-fundamentals.md %}
But if you think it could use more exposure we could consider including this text from elsewhere as well, and/or replacing some other things with it
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 4 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
yes exactly
OK - deleted!
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
"low latency reads and writes at the row levels" isn't actually the question, right? What even is a "read at the row level"? I think what we're trying to say is something along the lines of "Do I need that single region to be configurable at the row level?"
Makes sense. Revised to the below, let me know what you think
- Do I need that single region to be configurable at the row level? Or will the default region per table setting suffice?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 6 at r1 (raw file):
Previously, ajstorm (Adam Storm) wrote…
"...that must be read with low latency from all regions"?
Ack! Sorry, reverted to a non-technical use of "available" there
Updated to use Adam's text.
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 17 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: We might want to drop the part after "...aware of the location of nodes". Alternatively, it might be easier to read this if there was only one link per bullet.
Hmm how about
- The concept of locality, which CockroachDB uses to intelligently place and balance data based on how you define the following settings:
_includes/v21.1/topology-patterns/see-also.md, line 4 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: order is reversed in link.
Fixed!
_includes/v21.1/topology-patterns/see-also.md, line 14 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Do we want to say something about deprecating FTW?
Weird, I did not know it was deprecated. At least I'm not seeing any open docs issues that say "FTW is deprecated".
We do have an old PR about "deemphasizing" it (#8611) from like a year ago, but that's all I can find
Andy, FYI this may be a gap in the Product process if everyone knows this is deprecated but I can't find any Docs team work items for follow the workload with the word "deprecate" in them. :-)
Also, tbh I have no idea what the official company process is for saying something is "deprecated", which may be another process gap, or maybe I just missed it
Anyway should I file a docs issue to deprecate FTW across the docs? (Also I'd probably rather do that via a separate PR to follow, since there are multiple scattered docs that mention it that need updating)
v21.1/cockroach-demo.md, line 58 at r2 (raw file):
Previously, awoods187 (Andy Woods) wrote…
I think rich suggested we not delete it since someone might still want to see it as a comparison. Im fine deleting or keeping.
We still use it in way too many places. I don't think we can remove it in good faith until we have completely replaced it with something entirely else
v21.1/cost-based-optimizer.md, line 196 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Do we need this section at all anymore? Isn't it entirely obviated by GLOBAL tables?
I was going to ask about this under cover of a different issue. Went ahead and removed it and fixed up all the old incoming links as well
v21.1/disaster-recovery.md, line 164 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: Do we want to say here what that default setting is (zone survivability)?
Sure! Updated to say "zone-level survival goal"
v21.1/topology-follow-the-workload.md, line 15 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
nit: We're using the work "pattern" twice here.
Edited to remove use of "pattern" here altogether - now it says
"If read performance is your main focus for a table, but you want low-latency reads everywhere instead of just in the most active region, consider Global Tables or Follower Reads."
Also did some other edits on this page to remove the "pattern" term
v21.1/topology-follower-reads.md, line 15 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
+1
Rewrote this section to remove "pattern", and also all edited out other mentions of "pattern" on the page so it continues to fade ...
v21.1/topology-patterns.md, line 34 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: "not tied to geography" -> "not tied to a specific region"?
Updated both uses of "geography" to say "that are tied to a region" and "that are not tied to a region"
v21.1/topology-patterns.md, line 34 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: "that are geographically specific" -> "that are region specific"?
Updated!
v21.1/topology-patterns.md, line 37 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: not sure if I commented on this earlier, but it's a bit strange to me that we have the Resiliency column which is the same for every row. Can we have a Resiliency note at the bottom of this table which refers to the fact that Resiliency is set at the database level?
Yes! Bit of a mindless first pass there
Updated to be a single note at bottom that says
"In multi-region databases, the resiliency of each database depends on its survival goal settings."
v21.1/topology-patterns.md, line 40 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
At the risk of being a bit pedantic, writes will always be higher latency than reads, even for a single node database. Can we frame this instead as "Low-latency multi-region reads from all regions, at the expense of higher latency (cross-region) writes"?
Sounds much better to me! Updated
v21.1/topology-patterns.md, line 42 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
FTW is not compatible with the new syntax (I believe), so the survival goal entry in this row is incorrect.
It's removed now - per your previous comment, the survival goals text is in a note at the bottom of the table instead
v21.1/global-tables.md, line 12 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: specific regions?
Nit: Do we want to say "latency-sensitive reads"? Some people think of writes as SQL queries, even though they shouldn't.
Updated to say "specific regions" and changed "queries" to "reads"
v21.1/global-tables.md, line 38 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: this part strikes me as a bit strange, as we talk about the details, but never explain what those details are.
Agreed - deleted the second part of the sentence altogether, and re-worded it slightly to say:
"To use this pattern, you tell CockroachDB to set the table locality to GLOBAL."
v21.1/global-tables.md, line 107 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
It's a little unclear from reading this how Follower Reads relate to REGIONAL tables. I think we're going to want to clarify that. In this new world, stale follower reads are mostly a property of REGIONAL tables, because there's little reason to use them on GLOBAL tables.
Re: "there's little reason to use them on GLOBAL tables"
that makes sense to me. I deleted the line about follower reads from here since as you are saying they don't make any sense in this context
re: follower reads and REGIONAL tables
I think I will need to add some words to the docs in a separate PR. Still need to fully digest your comment in #9870 (comment) and incorporate that info in a PR that addresses that issue (and possibly #10377 and #10418 ). Does that make sense? (I'm not caught up yet on this much, so I may have misunderstood your comment)
v21.1/regional-tables.md, line 13 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: Might just be me, but I'm a bit worried that users might find the use of "geographies" confusing, as we only expose the concept of "regions". Is it possible to change it to "geographical region" and drop the "e.g."?
Updated to say only "region" and drop the e.g.
Probably a holdover from previous docs where we were more "free form/BYO zone configs" about geo things
v21.1/regional-tables.md, line 47 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Should we include steps for REGIONAL tables as well? We seem to jump right to REGIONAL BY ROW tables.
Wanted to do RBR since it's not the default and requires work, but added a note to the front that
"By default, all tables in a multi-region database are Regional tables. Therefore, the steps below show how to set up Regional by row tables."
v21.1/regional-tables.md, line 64 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: We could show that the table defaulted to
REGIONAL BY TABLEby runningSHOW CREATE TABLEhere.
Oh yeah - nice! Updated to add that in before we start munging the regions
v21.1/regional-tables.md, line 68 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
+1
Makes sense.
Revised to do it with UPDATE statements on crdb_region
v21.1/regional-tables.md, line 88 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
I find this part a bit odd, because we're explaining the zone config, but in a way that may only make sense to someone who already understands zone configurations (and in some detail). Also, we're not really explaining that the zone configuration is applied at each partition. My vote would be to remove this zone config description. To the point above about customers worrying that we moved their cheese, I'm not too concerned about that, since the hiding is the main point of this multi-region work.
Y'know this way of saying it makes sense to me - the zone config stuff is definitely a bit circular. Removed it altogether (here and on the global page) and updated the little note at the end to say "a good way" instead of the compare-y "a better way"
v21.1/regional-tables.md, line 92 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
"When rows are added or updated, which region the row is associated is specified as part of the update." - is this missing a "to"? Perhaps "When rows are added or updated, the region to which the row is associated is specified as part of the update."
Ended up deleting this zone config stuff per your other feedback
v21.1/regional-tables.md, line 124 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
For single region writes and reads?
This language is copied pretty much verbatim (unless I messed something up) from https://www.cockroachlabs.com/docs/v21.1/choosing-a-multi-region-configuration.html which was reviewed in #9900
That said I am happy to update it - are you saying we should just say "you get low latency for single-region writes and reads" here?
v21.1/regional-tables.md, line 126 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
...and low-latency multi-region stale reads from all other regions?
(FWIW - as above, language is from #9900)
Updated to add that clause! Seems important :-)
v21.1/regional-tables.md, line 130 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Nit: we might want to spell out AZ here.
Did it!
v21.1/regional-tables.md, line 136 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
...and the table is rarely modified?
Updated to add that!
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
Previously, awoods187 (Andy Woods) wrote…
showing zone configs is a quicker way to say "here's how you can verify this happened"
Yeah but then you have to explain both zone configs and how to read show zone configs which is very confusing and hard. I think we should do show create table. The contract should be like any other SQL, you see the SQL, you don't have to see what that translates into. I'd vote deleting but can disagree and committ.
Deleted the zone config stuff. I probably have some zone config dain bramage from working here pre-multi-region and it's hard to see through that sometimes :-)
As Adam explained below in a similar comment on the regional table page, the zone config stuff is pretty circular: it only makes sense if you already understand it. So I removed it altogether (here and on the regional page) and updated the little note at the end to say "a good way" instead of "a better way" so it fits
v21.1/topology-regional-tables.md, line 82 at r1 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Does it make sense to start with the standard RBR pattern, and have the TIMTOWTDI refer to a computed column example?
Makes sense. I ended up revising the section to use a simpler "set RBR, then do UPDATEs" flow. The multi-region latency demo has the fancy computed column method and we also link to some docs on crdb_region that show it, for those who like to get all compute-y with their columns
v21.1/topology-regional-tables.md, line 89 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
See reply above
FYI ended up deleting this Andy :-)
There was a problem hiding this comment.
Reviewed 17 of 17 files at r3.
Reviewable status:
_includes/sidebar-data-v21.1.json, line 659 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
This is a good question. There's a lot of overlap.
After some playing around I moved it under "Multi-Region Capabilities" as a subsection, since 5/6 of the pages are about "multi-region things" anyway
Let me know what you think. The "topology pattern" term is mostly gone from this section of the docs now, except in the name of that one overview page.
This organization looks good to me.
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Makes sense. Revised to the below, let me know what you think
- Do I need that single region to be configurable at the row level? Or will the default region per table setting suffice?
That all sounds good except for the term "the default". REGIONAL BY TABLE tables can be homed in regions other than the primary region, so I'm not sure what "default" means here.
Maybe something like: "Or will a single optimized region for the entire table suffice?".
Also, did we mean to drop the first sentence here?
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 17 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Hmm how about
- The concept of locality, which CockroachDB uses to intelligently place and balance data based on how you define the following settings:
I like that.
v21.1/global-tables.md, line 107 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Re: "there's little reason to use them on GLOBAL tables"
that makes sense to me. I deleted the line about follower reads from here since as you are saying they don't make any sense in this context
re: follower reads and REGIONAL tables
I think I will need to add some words to the docs in a separate PR. Still need to fully digest your comment in #9870 (comment) and incorporate that info in a PR that addresses that issue (and possibly #10377 and #10418 ). Does that make sense? (I'm not caught up yet on this much, so I may have misunderstood your comment)
Makes perfect sense, that can all be done separately. Don't mean to expand the scope of this PR even more.
v21.1/regional-tables.md, line 68 at r2 (raw file):
Revised to do it with UPDATE statements on crdb_region
Thanks.
Is this the right place to talk about auto-homing (the crdb_region column gets auto-assigned on insert) and locality optimized search (the local region is searched first on point lookups)? Asked another way, is this the right place to talk about how users should interact with REGIONAL BY ROW tables and what behavior they should expect from them?
There was a problem hiding this comment.
Revisions look great to me. One issue about FTW for @awoods187.
Reviewable status:
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 17 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
I like that.
SGTM too
_includes/v21.1/topology-patterns/see-also.md, line 14 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Weird, I did not know it was deprecated. At least I'm not seeing any open docs issues that say "FTW is deprecated".
We do have an old PR about "deemphasizing" it (#8611) from like a year ago, but that's all I can find
Andy, FYI this may be a gap in the Product process if everyone knows this is deprecated but I can't find any Docs team work items for follow the workload with the word "deprecate" in them. :-)
Also, tbh I have no idea what the official company process is for saying something is "deprecated", which may be another process gap, or maybe I just missed it
Anyway should I file a docs issue to deprecate FTW across the docs? (Also I'd probably rather do that via a separate PR to follow, since there are multiple scattered docs that mention it that need updating)
@awoods187 tagging you in for this one. Perhaps I went too far in saying that we're deprecating FTW, but AFAICT, it's not possible to achieve FTW with the new abstractions. If that's the case, isn't it implicitly deprecated if we routing everyone to the new abstractions?
v21.1/cockroach-demo.md, line 58 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
We still use it in way too many places. I don't think we can remove it in good faith until we have completely replaced it with something entirely else
OK, it lives another day...
v21.1/topology-patterns.md, line 37 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Yes! Bit of a mindless first pass there
Updated to be a single note at bottom that says
"In multi-region databases, the resiliency of each database depends on its survival goal settings."
Bueno!
v21.1/regional-tables.md, line 64 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Oh yeah - nice! Updated to add that in before we start munging the regions
Nice!
v21.1/regional-tables.md, line 88 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Y'know this way of saying it makes sense to me - the zone config stuff is definitely a bit circular. Removed it altogether (here and on the global page) and updated the little note at the end to say "a good way" instead of the compare-y "a better way"
Awesome!
v21.1/regional-tables.md, line 124 at r2 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
This language is copied pretty much verbatim (unless I messed something up) from https://www.cockroachlabs.com/docs/v21.1/choosing-a-multi-region-configuration.html which was reviewed in #9900
That said I am happy to update it - are you saying we should just say "you get low latency for single-region writes and reads" here?
Sorry, I wasn't clear. What I was suggesting was something like this, although admittedly, it's a bit wordy:
"...you get low latency for single-region writes and reads, as well as multi-region stale reads."
v21.1/topology-global-tables.md, line 69 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Deleted the zone config stuff. I probably have some zone config dain bramage from working here pre-multi-region and it's hard to see through that sometimes :-)
As Adam explained below in a similar comment on the regional table page, the zone config stuff is pretty circular: it only makes sense if you already understand it. So I removed it altogether (here and on the regional page) and updated the little note at the end to say "a good way" instead of "a better way" so it fits
We're all a bit drain bamaged, so don't feel bad. :-P
There was a problem hiding this comment.
Reviewable status:
_includes/v21.1/topology-patterns/see-also.md, line 14 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
@awoods187 tagging you in for this one. Perhaps I went too far in saying that we're deprecating FTW, but AFAICT, it's not possible to achieve FTW with the new abstractions. If that's the case, isn't it implicitly deprecated if we routing everyone to the new abstractions?
We aren't deprecating FTW--its the only non-enterprise method of approximating multi-region. Adam is correct that FTW does not work with the new MR abstractions tho so we should clarify that.
|
LGTM--very well done! |
rmloveland
force-pushed
the
20210422-multiregion-capabilities
branch
from
3beaca4
to
25b048f
Compare
There was a problem hiding this comment.
Thanks guys for the careful and thorough reviews! This is definitely way better as a result.
Nathan, I think I have addressed your remaining comments - PTAL!
Reviewable status:
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
That all sounds good except for the term "the default". REGIONAL BY TABLE tables can be homed in regions other than the primary region, so I'm not sure what "default" means here.
Maybe something like: "Or will a single optimized region for the entire table suffice?".
Also, did we mean to drop the first sentence here?
Weird, no I did not mean to drop that. Dug it out of the git bucket and added it back
Whole line now reads per your suggested text
"- Do I need low-latency reads and writes from a single region? Do I need that single region to be configurable at the row level? Or will a single optimized region for the entire table suffice?"
_includes/v21.1/topology-patterns/see-also.md, line 14 at r2 (raw file):
Previously, awoods187 (Andy Woods) wrote…
We aren't deprecating FTW--its the only non-enterprise method of approximating multi-region. Adam is correct that FTW does not work with the new MR abstractions tho so we should clarify that.
OK, filed #10520 to add that info to the docs
v21.1/global-tables.md, line 107 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Makes perfect sense, that can all be done separately. Don't mean to expand the scope of this PR even more.
👍
v21.1/regional-tables.md, line 68 at r2 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
Revised to do it with UPDATE statements on crdb_region
Thanks.
Is this the right place to talk about auto-homing (the crdb_region column gets auto-assigned on insert) and locality optimized search (the local region is searched first on point lookups)? Asked another way, is this the right place to talk about how users should interact with REGIONAL BY ROW tables and what behavior they should expect from them?
Good idea.
I added the following things (the locality optimized search paragraph is an included fragment, since it was first added in #10280). Also added the locality optimized search info to the ALTER TABLE ... SET LOCALITY doc so folks can find it there too
By default, the region column will get auto-assigned on insert; this is also known as "auto-homing". For more information about how the crdb_region column works, see ALTER TABLE ... SET LOCALITY REGIONAL BY ROW.
Note that there is a performance benefit for queries that select a single row (e.g., SELECT * FROM users WHERE email = 'anemailaddress@gmail.com'). If 'anemailaddress@gmail.com' is found in the local region, there is no need to search remote regions. This feature, whereby the SQL engine will avoid sending requests to nodes in other regions when it can read a value from a unique column that is stored locally, is known as locality optimized search.
v21.1/regional-tables.md, line 124 at r2 (raw file):
Previously, ajstorm (Adam Storm) wrote…
Sorry, I wasn't clear. What I was suggesting was something like this, although admittedly, it's a bit wordy:
"...you get low latency for single-region writes and reads, as well as multi-region stale reads."
Sounds good - updated!
|
Files changed: _includes/sidebar-data-v21.1.json |
There was a problem hiding this comment.
These docs look great, Rich! Awesome job (as always).
I just had a few nits/suggestions that you can very much take or leave. LGTM.
(BTW, the number of lines deleted in this PR warms my soul.)
Reviewed 21 of 38 files at r1, 4 of 13 files at r2, 15 of 17 files at r3, 5 of 5 files at r4.
Reviewable status:
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r4 (raw file):
nit
capabilities ... handles
capabilities ... handle
_includes/v21.1/topology-patterns/multiregion-db-setup.md, line 1 at r4 (raw file):
suggestion:
use it:
set it as the default database:
v21.1/computed-columns.md, line 77 at r4 (raw file):
/computed-columns/partitioning.md
Should we remove this file?
v21.1/topology-follow-the-workload.md, line 6 at r4 (raw file):
toc: true ---
I was going to say something about adding a note here about compatibility with the new syntax, but I see you also have #10520...
Maybe it's still worth just adding a note here for now? IDK. Up to you.
v21.1/topology-patterns.md, line 53 at r4 (raw file):
- Single-region deployments using 2 zones, or multi-region deployments using 2 regions. In these cases, the cluster would be unable to survive the loss of a single zone or a single region, respectively. - Broadly distributed multi-region deployments (e.g., `us-west`, `asia`, and `europe`) using only the default [Follow-the-Workload](topology-follow-the-workload.html) behavior. In this case, latency will likely be unacceptably high.
Suggestion:
It might be helpful to suggest a topology pattern or subset of topology patterns to use instead.
For example:
... unacceptably high. Instead, use a [Regional Tables](regional-tables.html) or [Global Tables](global-tables.html) pattern that best fits your use-case.
v21.1/global-tables.md, line 2 at r4 (raw file):
nit:
The Global Table Locality Pattern
The other titles in this ToC all have "Topology Pattern" in the title, and none of them start with "The". IDK if it's worth revisiting these titles - just a thing to think about. Same thing for the Regional Table page title.
v21.1/global-tables.md, line 33 at r4 (raw file):
suggestion:
{{site.data.alerts.callout_info}} `GLOBAL` tables (and the other [multi-region capabilities](multiregion-overview.html)) require an [Enterprise license](https://www.cockroachlabs.com/get-cockroachdb). {{site.data.alerts.end}}
IMO, this should be at the very top of the page.
Same thing for the Regional Table page.
Fixes #9268 Fixes #9266 Fixes #9265 Fixes #9264 Fixes #10425 Fixes #10319 Addresses #10051 Addresses #10401 Addresses #10461 Addresses #10463 Summary of changes: - Update "topology patterns" docs as follows: - Move them as a sub-section under "Multi-Region Capabilities", since they are mostly relevant to multi-region usage - Deemphasize "topology" for these patterns, since these are not network topologies, they are mostly per-table configurable behaviors - Add "global tables" to replace the "duplicate indexes" page - Add "regional tables" to replace the "geo-partitioned XYZ" pages - Fix up ~all links in the docs to point to the new things - Update mentions of partitioning throughout the docs to note that most users should use the new v21.1+ multi-region capabilities instead of explicit partitioning - Also removed mention of partitioning from several places as part of deemphasing explicit partitioning generally, since most use cases are covered by multi-region abstractions - Update the 'Cost-Based Optimizer' page to remove the old skool "nearest index" partitioning stuff, since now we have GLOBAL tables - This required removing links to the above from all the "enterprise features" lists and replacing with links to the new 'Multi-Region Capabilities' page - Update `cockroach demo` to deemphasize partitioning and point to new multi-region things - Update disaster recovery pages to mention most users of new deployments should just use multi-region survival goals - Update various licensing docs to point to the new multi-region things - Update replication reports to mention the new multi-region things, in lieu of further updates - Renamed a bunch of links that used the phrase "geo-partitioning" to refer to the multi-region latency tutorial, which now goes by the name "Multi-Region Performance" - Updated 'ALTER TABLE ... SET LOCALITY REGIONAL BY ROW' to include info about locality optimized search - Note: we explicitly did *not* touch anything related to the multi-region Flask app, since that work is happening via #10394
rmloveland
force-pushed
the
20210422-multiregion-capabilities
branch
from
25b048f
to
f89ebe4
Compare
There was a problem hiding this comment.
Thanks Eric!!!!1! Made almost all of your suggested updates, thanks for the comments
And yes deleting stuff is my fave also :-)
Reviewable status:
_includes/v21.1/sql/use-multiregion-instead-of-partitioning.md, line 2 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
nit
capabilities ... handlescapabilities ... handle
Fixed
_includes/v21.1/topology-patterns/multiregion-db-setup.md, line 1 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
suggestion:
use it:set it as the default database:
Fixed!
v21.1/computed-columns.md, line 77 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
/computed-columns/partitioning.mdShould we remove this file?
oooooh good eye! deleted with EXTREME PREJUDICE
v21.1/topology-follow-the-workload.md, line 6 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
I was going to say something about adding a note here about capability with the new syntax, but I see you also have #10520...
Maybe it's still worth just adding a note here for now? IDK. Up to you.
Gonna do a separate PR because we mention FTW in a bunch of places and I want to catch em all Pokemon style, and this PR is already ready for middle school 😆
v21.1/topology-patterns.md, line 53 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
Suggestion:
It might be helpful to suggest a topology pattern or subset of topology patterns to use instead.
For example:
... unacceptably high. Instead, use a [Regional Tables](regional-tables.html) or [Global Tables](global-tables.html) pattern that best fits your use-case.
Y'know, this is probably a good idea, but I'm gonna pass for now. Everything has already been vetted on the technical side and I don't want to add any new recommendations since I need to finish this one up and move on
v21.1/global-tables.md, line 2 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
nit:
The Global Table Locality PatternThe other titles in this ToC all have "Topology Pattern" in the title, and none of them start with "The". IDK if it's worth revisiting these titles - just a thing to think about. Same thing for the Regional Table page title.
Good point. I simplified the titles for these pages to be "Global Tables" and "Regional Tables", respectively.
v21.1/global-tables.md, line 33 at r4 (raw file):
Previously, ericharmeling (Eric Harmeling) wrote…
suggestion:
{{site.data.alerts.callout_info}} `GLOBAL` tables (and the other [multi-region capabilities](multiregion-overview.html)) require an [Enterprise license](https://www.cockroachlabs.com/get-cockroachdb). {{site.data.alerts.end}}IMO, this should be at the very top of the page.
Same thing for the Regional Table page.
Agreed. Moved 'em up!
|
Files changed: _includes/sidebar-data-v21.1.json |
There was a problem hiding this comment.
Reviewed 1 of 17 files at r3, 4 of 5 files at r4, 6 of 6 files at r5.
Reviewable status:
_includes/v21.1/topology-patterns/multiregion-fundamentals.md, line 5 at r1 (raw file):
Previously, rmloveland (Rich Loveland) wrote…
Weird, no I did not mean to drop that. Dug it out of the git bucket and added it back
Whole line now reads per your suggested text
"- Do I need low-latency reads and writes from a single region? Do I need that single region to be configurable at the row level? Or will a single optimized region for the entire table suffice?"
Looks great now.
Fixes #9268
Fixes #9266
Fixes #9265
Fixes #9264
Fixes #10425
Fixes #10319
Addresses #10051
Addresses #10401
Addresses #10461
Addresses #10463
Summary of changes:
Update multi-region "topology patterns" as follows:
Deemphasize "topology" for these patterns, since it really isn't -
it's per-table
Add "global tables" as a pattern
Add "regional tables" as a pattern
Remove "duplicate indexes" and "geo-partitioned foos" pages and have
them redirect to the above, as appropriate
Fix up ~all links in the docs to point to the new things
Update mentions of partitioning throughout the docs to note that most
users should use new v21.1+ multi-region capabilities instead of
explicit partitioning
deemphasing explicit partitioning generally, since most use cases
are covered by MR abstractions
Update
cockroach demoto deemphasize partitioning and point to newMR things
Update disaster recovery pages to mention most users of new
deployments should just use multi-region survival goals
Update various licensing docs to point to the new multi-region things
Update replication reports to mention the new MR things, in lieu of
further updates
Renamed a bunch of links that used the phrase "geo-partitioning" to
refer to the multi-region latency tutorial, which now goes by the name
"Multi-Region Performance"
Note: we explicitly did not touch anything related to the
multi-region Flask app, since that work is happening via v21.1 movr-flask updates #10394