21.1 multi-region #27
Conversation
|
@otan, @rmloveland - added y'all as reviewers, but no rush/feel free to pass to someone else. I think the main thing I need reviewed is |
dbinit.sql
Outdated
| ; | ||
| CONSTRAINT fk_city_ref_users FOREIGN KEY (rider_id) REFERENCES users(id), | ||
| CONSTRAINT fk_vehicle_ref_vehicles FOREIGN KEY (vehicle_id) REFERENCES vehicles(id) | ||
| ); |
There was a problem hiding this comment.
After fixing the table (see comment below), I formatted the document with pgformatter. Should look nicer all over now.
dbinit.sql
Outdated
| ); | ||
|
|
||
|
|
||
| ALTER TABLE rides ADD COLUMN region crdb_internal_region AS ( |
There was a problem hiding this comment.
do we ever make this table REGIONAL BY ROW? seems like it is REGIONAL BY TABLE at the moment.
we should do this inline, i.e.
CREATE TABLE users (
...,
region crdb_internal_region CASE WHEN city = 'amsterdam' THEN 'gcp-europe-west1'
WHEN city = 'paris' THEN 'gcp-europe-west1'
WHEN city = 'rome' THEN 'gcp-europe-west1'
WHEN city = 'new york' THEN 'gcp-us-east1'
WHEN city = 'boston' THEN 'gcp-us-east1'
WHEN city = 'washington dc' THEN 'gcp-us-east1'
WHEN city = 'san francisco' THEN 'gcp-us-west1'
WHEN city = 'seattle' THEN 'gcp-us-west1'
WHEN city = 'los angeles' THEN 'gcp-us-west1'
ELSE 'gcp-us-east1',
....
) LOCALITY REGIONAL BY ROW; -- optionally REGIONAL BY ROW AS "region"
same with the others
There was a problem hiding this comment.
My mistake. Fixed for rides.
Also, moved the region column definition to the CREATE TABLE statement for all tables.
dbinit.sql
Outdated
| @@ -4,130 +4,97 @@ SET sql_safe_updates = false; | |||
| DROP DATABASE IF EXISTS movr CASCADE; | |||
|
|
|||
|
|
|||
| CREATE DATABASE movr; | |||
| CREATE DATABASE movr PRIMARY REGION "gcp-us-east1" REGIONS "gcp-europe-west1", "gcp-us-west1"; | |||
There was a problem hiding this comment.
the problem with this setup is that users have to ensure they set up the servers correctly, i.e. set --locality=region=<blah> in their flask setup. is that what we want / already have?
There was a problem hiding this comment.
Yeah ... the README and tutorial specifically instruct the user to specify the localities that match these region names. They are based on the GCP cluster names. On CC, the localities for a multi-region GCP cluster should match these names anyways.
|
TFTR @otan ! |
There was a problem hiding this comment.
LGTM
As far as I understand what is happening here, is the summary basically the following?
- Update to use the new multi-region SQL abstractions instead of those yucky partitioning stmts
- Use a version of movr underneath that does not have the compound PK on (city, id)
|
TFTR @rmloveland !
Exactly. I also snuck in the fixed |
Fixes #25.
Fixes #4.