-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Materialize workflow support for reference tables #16787
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
4ea1434
to
cdfa6b3
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16787 +/- ##
==========================================
- Coverage 69.51% 69.50% -0.02%
==========================================
Files 1569 1569
Lines 202517 202529 +12
==========================================
- Hits 140780 140761 -19
- Misses 61737 61768 +31 ☔ View full report in Codecov by Sentry. |
55d06cd
to
f915cc0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
go/cmd/vtctldclient/command/vreplication/materialize/materialize.go
Outdated
Show resolved
Hide resolved
go/cmd/vtctldclient/command/vreplication/materialize/materialize.go
Outdated
Show resolved
Hide resolved
df782f3
to
deedc2a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to mention this in the release notes as well, once the CLI options are final.
case common.CreateOptions.IsReference && len(common.CreateOptions.Tables) == 0: | ||
return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "cannot specify --reference without --tables") | ||
case !common.CreateOptions.IsReference && len(common.CreateOptions.Tables) > 0: | ||
return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "cannot specify --tables without --reference") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If both options are required, why not replace them with a single --reference-tables
option? It will still be mutually exclusive with --table-settings
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had kept them as two options as
--tables
felt more consistent as we have a simlarMoveTables
option- In case we would have other use cases similar to
--reference
where we would materialize a list of special tables.
But I don't really have a clear scenario in mind. Will switch to the single option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified to use single option. Added to release notes.
changelog/21.0/21.0.0/summary.md
Outdated
There is a new option in `Materialize` workflows to keep a synced copy of reference tables from the unsharded keyspace | ||
which holds the source of truth for the reference table, to all shards in a sharded keyspace. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a new option in `Materialize` workflows to keep a synced copy of reference tables from the unsharded keyspace | |
which holds the source of truth for the reference table, to all shards in a sharded keyspace. | |
There is a new option in [`Materialize` workflows](https://vitess.io/docs/reference/vreplication/materialize/) to keep a synced copy of [reference or lookup tables](https://vitess.io/docs/reference/vreplication/reference_tables/) (countries, states, zip_codes, etc) from an unsharded keyspace which holds the source of truth for the reference table, to all shards in a sharded keyspace. |
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
…shards in a target keyspace Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
…tables. Self-review. Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
Signed-off-by: Rohit Nayak <rohit@planetscale.com>
ab82464
to
8af9387
Compare
Description
This PR adds support for materializing reference tables by adding options to the
Materialize
workflow. Currently the user needs to specify the target table info along with the materialize query using the--table_settings
flag which takes a json spec.Two new flags
--reference
(bool) and--tables
(csv list) have been added. Reference table materialization cannot be combined with regularMaterialize
workflows. It is one or the other. Reference table materializing just requires setting up a copy of the tables into all target shards.Example:
Materialize --target-keyspace ks2 --workflow wf1 create --source-keyspace ks1 --reference --tables ref1,ref2
Doc PR: vitessio/website#1849
Related Issue(s)
Checklist