-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
configprofiles: emphasize "cluster virtualization" #106116
Conversation
78f940e
to
f1c6c52
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.
Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @knz and @stevendanna)
pkg/configprofiles/profiles.go
line 35 at r1 (raw file):
description: "configuration suitable for a replication target cluster", }, "multitenant+app+sharedservice+repl": {aliasTarget: "virtual+app+sharedservice+repl", hidden: true},
What's the thinking behind keeping these two particular aliases? Is it for backwards-compatibility? Are the config profiles being used somewhere so that we need backwards-compatibility?
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @stevendanna and @yuzefovich)
pkg/configprofiles/profiles.go
line 35 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
What's the thinking behind keeping these two particular aliases? Is it for backwards-compatibility? Are the config profiles being used somewhere so that we need backwards-compatibility?
Let's simplify. Thanks.
f1c6c52
to
ed1714b
Compare
Before this patch: ``` $ cockroach start-single-node --config-profile=help ... replication-source configuration suitable for a replication source cluster (alias for "multitenant+app+sharedservice+repl") replication-target configuration suitable for a replication target cluster (alias for "multitenant+noapp+repl")` multitenant+app+sharedservice multi-tenant cluster with one secondary tenant configured to serve SQL application traffic multitenant+app+sharedservice+repl multi-tenant cluster with one secondary tenant configured to serve SQL application traffic, with replication enabled multitenant+noapp multi-tenant cluster with no secondary tenant defined yet multitenant+noapp+repl multi-tenant cluster with no secondary tenant defined yet, with replication enabled ``` After this patch: ``` replication-source configuration suitable for a replication source cluster (alias for "virtual+app+sharedservice+repl") replication-target configuration suitable for a replication target cluster (alias for "virtual+noapp+repl") virtual+app+sharedservice one virtual cluster configured to serve SQL application traffic virtual+app+sharedservice+repl one virtual cluster configured to serve SQL application traffic, with replication enabled virtual+noapp virtualization enabled but no virtual cluster defined yet virtual+noapp+repl virtualization enabled but no virtual cluster defined yet, with replication enabled ``` Release note: None
ed1714b
to
8b7f893
Compare
TFYR bors r=yuzefovich |
Build succeeded: |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 8b7f893 to blathers/backport-release-23.1-106116: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Informs #106068.
Epic: CRDB-29380
Before this patch:
After this patch:
Release note: None