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

RESTORE and IMPORT INTO multi-region functionality docs #11966

Merged
merged 2 commits into from
Nov 3, 2021

Conversation

kathancox
Copy link
Contributor

Closes #10937 Draft of features for RESTORE & IMPORT INTO 21.2 multi-region features:

RESTORE doc changes:

IMPORT INTO doc changes:

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@github-actions
Copy link

github-actions bot commented Oct 14, 2021

Files changed:

_includes/v21.2/backups/no-multiregion-table-backups.md
_includes/v21.2/known-limitations/rbr-restore-no-support.md
_includes/v21.2/known-limitations/restore-multiregion-match.md
_includes/v21.2/known-limitations/restore-tables-non-multi-reg.md
v21.2/import-into.md
v21.2/import.md
v21.2/known-limitations.md
v21.2/restore.md
v21.2/set-locality.md

@netlify
Copy link

netlify bot commented Oct 14, 2021

✔️ Netlify Preview

🔨 Explore the source changes: 8a6fc16

🔍 Inspect the deploy log: https://app.netlify.com/sites/cockroachdb-docs/deploys/6182e8b6ecc70d0007562e16

😎 Browse the preview: https://deploy-preview-11966--cockroachdb-docs.netlify.app

@kathancox
Copy link
Contributor Author

@livlobo I think Paul and Ellie worked on quite a few of the PRs

@kathancox kathancox requested a review from dt October 19, 2021 20:17
@kathancox
Copy link
Contributor Author

@livlobo @dt @adityamaru Liv has just opened #12108 the changes in which will be live in a patch release. Can we prioritize reviewing this PR for the GA release in a couple of weeks and then the enhancements to MR backup and table restores I can start work on — once this is merged — ready for the patch release.

~~~ sql
EXPORT INTO CSV 'nodelocal://0/src_rbr' FROM SELECT crdb_region, i from src_rbr;
~~~
### Using `RESTORE` with multi-region table localities
Copy link

Choose a reason for hiding this comment

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

We should also add that restoring global/RBT tables into a non-MR DB is not supported + tracking issue: backupccl: allow restore of GLOBAL/RBT tables in a non-MR database #71502

And in the Backup limitations page of MR -- we should add that: Table level backups are NOT supported + tracking issue: backupccl: support for multi-region table backups #71180

Copy link

@livlobo livlobo left a comment

Choose a reason for hiding this comment

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

LGTM, Kathryn - subject to update to the 2 additional known limitations.

@kathancox kathancox force-pushed the multi-region-bulk-features branch from 88e1538 to 3b3f018 Compare November 1, 2021 16:04
@kathancox kathancox removed the request for review from dt November 3, 2021 12:59
@@ -0,0 +1 @@
Restoring [`REGIONAL BY ROW`](multiregion-overview.html#regional-by-row-tables) tables is currently not supported. [Tracking GitHub Issue](https://github.com/cockroachdb/cockroach/pull/71178)

Choose a reason for hiding this comment

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

Should the issue be this cockroachdb/cockroach#67269?

@kathancox kathancox force-pushed the multi-region-bulk-features branch from 3b3f018 to 8a307b0 Compare November 3, 2021 14:30
@kathancox kathancox requested a review from rmloveland November 3, 2021 14:39
Copy link
Contributor

@rmloveland rmloveland left a comment

Choose a reason for hiding this comment

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

LGTM, with some opinions about a few things


The following must also be true for `RESTORE` to be successful:

* The regions of the source database and the regions of the destination database have the same set of regions.
Copy link
Contributor

Choose a reason for hiding this comment

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

First mention of 'regions' could link to our docs on database regions: https://www.cockroachlabs.com/docs/dev/multiregion-overview.html#database-regions

The following must also be true for `RESTORE` to be successful:

* The regions of the source database and the regions of the destination database have the same set of regions.
* The regions were added to each of the databases in the same order.
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting .. .this is gonna be a tough one for many users to know, I think


* The regions of the source database and the regions of the destination database have the same set of regions.
* The regions were added to each of the databases in the same order.
* The databases have the same primary region.
Copy link
Contributor

Choose a reason for hiding this comment

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

> ALTER DATABASE destination_database ADD region "us-east1";
~~~

Furthermore, this scenario has mismatched regions between the databases since the regions were not added to the database in the same order.
Copy link
Contributor

Choose a reason for hiding this comment

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

I would find it clearer if this said something like "In addition, the following scenario ..." for the following reasons:

  1. "furthermore" makes it sound like this is a continuation of the previous example
  2. "this" is somewhat ambiguous because it makes me think it could be referring to the previous example

v21.2/import.md Outdated
@@ -22,7 +22,7 @@ The `IMPORT` [statement](sql-statements.html) imports the following types of dat
- `IMPORT` cannot be used with [user-defined types](create-type.html). Use [`IMPORT INTO`](import-into.html) instead.
- `IMPORT` is a blocking statement. To run an import job asynchronously, use the [`DETACHED`](#options-detached) option.
- `IMPORT` cannot be used within a [rolling upgrade](upgrade-cockroach-version.html).
- {% include {{page.version.version}}/sql/import-into-regional-by-row-table.md %}
- `IMPORT` cannot directly import data to `REGIONAL BY ROW` tables that are part of [multi-region databases](multiregion-overview.html). Instead, use [`IMPORT INTO`](import-into.html) which now supports importing into `REGIONAL BY ROW` tables.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest removing the already implied "now" (it's always now on the internet)

if you want to call out the timeliness aspect of this being a new thing in v21.2, maybe an edit like

"IMPORT cannot directly .... new in v21.2: Instead, use IMPORT INTO which supports importing into REGIONAL BY ROW tables"

also, FWIW as an outsider I find the IMPORT vs IMPORT INTO distinction quite confusing - esp if feature sets are different. nothing to do from the POV of this PR however ....

v21.2/restore.md Outdated

* A cluster's regions will be checked before a restore. Mismatched regions between [backup](known-limitations.html#backup-of-multi-region-tables) and restore clusters will be flagged before the restore begins, which allows for a decision between updating the [cluster localities](cockroach-start.html#locality) or restoring with the [`skip_localities_check`](#skip-localities-check) option to continue with the restore regardless.

* A database that is restored with the `sql.defaults.primary_region` [cluster setting](cluster-settings.html) will have the `PRIMARY REGION` from this cluster setting assigned to the target database.
Copy link
Contributor

Choose a reason for hiding this comment

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

v21.2/restore.md Outdated

* `RESTORE` supports restoring **non**-multi-region tables into a multi-region database and sets the table locality as [`REGIONAL BY TABLE`](multiregion-overview.html#regional-tables) to the primary region of the target database.

* Restoring tables from multi-region databases with table localities set to `REGIONAL BY TABLE`, [`REGIONAL BY TABLE IN PRIMARY REGION`](set-locality.html#regional-by-table), and [`GLOBAL`](set-locality.html#global) to another multi-region database is supported. Note that when restoring a `REGIONAL BY TABLE IN PRIMARY REGION` table, if the primary region is different in the source database to the target database this will be implicitly changed on restore.
Copy link
Contributor

Choose a reason for hiding this comment

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

The "Note that ..." caveat seems important and easy to miss, may be worth breaking into a nested bullet under this one for the Buzzfeed listicle scanners like me

The ordering of regions and how region matching is determined is a known limitation. See the [Known Limitations](#known-limitations) section for the tracking issues on limitations around `RESTORE` and multi-region support.

For more on multi-region databases, see the [Multi-Region Capabilities Overview](multiregion-overview.html).

Copy link
Contributor

Choose a reason for hiding this comment

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

PS these docs are awesome (in case I forgot to say earlier)

v21.2/restore.md Outdated
## Known limitations

* {% include {{ page.version.version }}/known-limitations/restore-aost.md %} [Tracking GitHub Issue](https://github.com/cockroachdb/cockroach/issues/53044)
* To successfully [restore a table into a multi-region database](#restoring-to-multi-region-databases), it is necessary for the order and regions to match between the source and destination database. [Tracking GitHub Issue](https://github.com/cockroachdb/cockroach/issues/71071)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this one could benefit from linking to your more full explanation on the Known Limitations page. If I hadn't read that first I would not know what "the order ... to match" means

@@ -36,6 +36,10 @@ The user must be a member of the [`admin`](authorization.html#roles) or [owner](

## Examples

{{site.data.alerts.callout_info}}
[`RESTORE`](restore.html) on [`REGIONAL BY TABLE`](#regional-by-table) and [`GLOBAL`](#global) tables is supported with some limitations — see [Restoring to multi-region databases](restore.html#restoring-to-multi-region-databases) for more detail. Tables set to a [`REGIONAL BY ROW`](#regional-by-row) table locality cannot be restored.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth breaking these sentences with a newline between them since one thing is saying "you can" and one is saying "you cannot"

Also think it's worth adding a mention and link to the full Known Limitations page section from the "Tables set to RBR cannot be restored" part

@kathancox kathancox merged commit f2780e4 into master Nov 3, 2021
@kathancox kathancox deleted the multi-region-bulk-features branch November 3, 2021 20:19
@kathancox
Copy link
Contributor Author

TFTR @rmloveland !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants