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.

Sign up for GitHub

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

Jump to bottom

sql: create_tenant cluster bootstrapping must be version-gated #94773

Closed
postamar opened this issue Jan 5, 2023 · 1 comment
Closed

sql: create_tenant cluster bootstrapping must be version-gated #94773

postamar opened this issue Jan 5, 2023 · 1 comment
Assignees
@postamar
Labels
A-multitenancy Related to multi-tenancy branch-master Failures and bugs on the master branch. branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-shared-systems Shared Systems Team

Comments

@postamar
Copy link
Contributor

postamar commented Jan 5, 2023 •
edited by exalate-issue-sync bot
Loading

Presently crdb_internal.create_tenant bootstraps a cluster using the values generated by the current binary. It then sets the tenant cluster's version to that of the host cluster's active version.

The problem is that in a mixed-version host cluster, when a node running the binary for the latest release executes this function:

  • the tenant cluster is bootstrapped using the values for the latest release;
  • the upgrades, which may include system schema migrations, have yet to run.

While a single upgrade step is idempotent, there's no expectation for a set of upgrade steps to be idempotent, so the upgrade may fail. Furthermore, the tenant cluster may be inoperable in the face of version gates for versions corresponding to system schema migrations: the gated behaviour expects the system schema to be in the "old" state but in fact it's already in the "new" state.

For this purpose, CRDB binaries need to be able to bootstrap tenant clusters using the same values as the binary of the previous major release would.

Jira issue: CRDB-23136

Epic CRDB-10829
@postamar postamar added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 branch-master Failures and bugs on the master branch. labels Jan 5, 2023
@postamar postamar mentioned this issue Jan 5, 2023
Merged
@postamar postamar mentioned this issue Jan 5, 2023
Merged
@postamar postamar mentioned this issue Jan 5, 2023
Merged
@postamar postamar self-assigned this Jan 5, 2023
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
ad0ff7e 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
sql: fix mixed-version behaviour of create_tenant()
a56de4c 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
240eef0 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
a26c6bc 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
sql: fix mixed-version behaviour of create_tenant()
37d302c 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
2ae9c65 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
37dac2a 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
sql: fix mixed-version behaviour of create_tenant()
228f74d 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
29aef80 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
sql: fix mixed-version behaviour of create_tenant()
64a402f 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 6, 2023
bootstrap: add string representation of initial values
aa19129 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
jeffswenson pushed a commit to jeffswenson/cockroach that referenced this issue Jan 9, 2023
@jeffswenson
bootstrap: add string representation of initial values
8709ca1 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
jeffswenson pushed a commit to jeffswenson/cockroach that referenced this issue Jan 9, 2023
@jeffswenson
sql: fix mixed-version behaviour of create_tenant()
b7645b2 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
@postamar postamar added do-not-merge bors won't merge a PR with this label. and removed do-not-merge bors won't merge a PR with this label. labels Jan 9, 2023
@exalate-issue-sync exalate-issue-sync bot added the T-shared-systems Shared Systems Team label Jan 10, 2023
@knz knz added the A-multitenancy Related to multi-tenancy label Jan 11, 2023
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 12, 2023
bootstrap: add string representation of initial values
8f4135b 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 12, 2023
sql: fix mixed-version behaviour of create_tenant()
3db94bb 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 12, 2023
bootstrap: add string representation of initial values
481e712 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 12, 2023
sql: fix mixed-version behaviour of create_tenant()
8722605 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 20, 2023
bootstrap: add string representation of initial values
713b512 
This commit adds InitialValuesToString and InitialValuesFromString which
convert KV datums and KV split keys from a MetadataSchema to a string
representation and vice-versa. The tenant prefix is omitted from the
string representation.

This functionality exists to allow bootstrapping clusters using the
values and splits as generated by an earlier cockroach binary.

Informs cockroachdb#94773.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jan 20, 2023
sql: fix mixed-version behaviour of create_tenant()
08f2dc4 
Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs cockroachdb#94773.

Release note: None
craig bot pushed a commit that referenced this issue Jan 21, 2023
@dt @adityamaru
Merge #94774 #95355 #95528
1982295 
94774: sql: fix mixed-version behaviour of create_tenant() r=postamar a=postamar

Previously, running create_tenant on the latest binary in
a mixed-version cluster would bootstrap the system schema using the
logic from the latest binary, in which all system database migration
upgrade steps have been "baked in".

This might not be compatible with the the tenant cluster's version which
is (correctly) set to the active host cluster version, the idea there
being that tenant clusters versions cannot be greater than host cluster
versions.

This commit fixes this by version-gating the tenant cluster system
schema bootstrapping, and using hard-coded values when bootstrapping
into the old version.

Informs #94773.

Release note: None

95355: server: always enable sql_instances maintenance r=knz a=dt

Fixes #95571.

Previously the system.sql_instances table was only maintained by SQL servers that were operating in "pod" mode, i.e. not in mixed KV and SQL process nodes, where KV-level liveness and gossip provides an alternative means of node discovery that can be used by the SQL layer when searching for other SQL instances. However this inconsistency makes writing correct remote-node discovery and interaction SQL-level code difficult: in some cases such code needs to consult the instances list, and in some the KV liveness store, which when combined with complexities of doing so around initialization, dependency-injection, etc can become hard to maintain.

Additionally such a design precludes a cluster where some SQL instances are in mixed KV nodes and some are not, as the non-KV nodes would have no way discover the KV ones. Such deployments are not currently possible but could be in the future.

Instead, this change enabled maintenance of the sql_instances table by all SQL servers, whether running in their own processes or embedded in a KV storage node process. This paves the way for making the means of discovery of SQL servers uniform across all SQL server types: they will all be able to simply consult the instances list, to find any other SQL servers, regardless of where those SQL servers are running.

A follow-up change could simplify DistSQLPhysicalPlanner, specifically the SetupAllNodesPlanning method that has two different implementations due to the previous inconsistency in the available APIs.

Release note: none.
Epic: CRDB-14537


95528: backupccl: fix flaky TestExcludeDataFromBackupAndRestore r=msbutler a=adityamaru

We don't need to wait for the table to split, inspecting the state of the leaseholders replica is adequate and a more correct source of truth to rely on.

In some cases the test would not wait for `data.bar` to split into its own range and so it would incorrectly be excluded from the backup resulting in 0 rows instead of 10 in the final assertion.

Fixes: #95350

Release note: None

Co-authored-by: Marius Posta <[email protected]>
Co-authored-by: David Taylor <[email protected]>
Co-authored-by: adityamaru <[email protected]>
@postamar
Copy link
Contributor Author

I think this can be closed now.

@postamar postamar mentioned this issue Feb 1, 2023
Merged
postamar pushed a commit to postamar/cockroach that referenced this issue Feb 1, 2023
logictest: add mixed-version tenant bootstrap test case
0c906de 
This commit enhances the testserver_upgrade_node logic test suite with
cases that verify that in a mixed-version cluster, tenant creation
bootstraps the same data in the tenant span regardless of which
executable is currently performing it. Until the upgrade is finalized,
the executables for the new release must behave exactly like those for
the old release.

Informs cockroachdb#94773.

Release note: None
craig bot pushed a commit that referenced this issue Feb 2, 2023
Merge #96379
8c216e1 
96379: logictest: add mixed-version tenant bootstrap test case r=postamar a=postamar

This commit enhances the testserver_upgrade_node logic test suite with cases that verify that in a mixed-version cluster, tenant creation bootstraps the same data in the tenant span regardless of which executable is currently performing it. Until the upgrade is finalized, the executables for the new release must behave exactly like those for the old release.

Informs #94773.

Release note: None

Co-authored-by: Marius Posta <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@postamar postamar

Labels
A-multitenancy Related to multi-tenancy branch-master Failures and bugs on the master branch. branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-shared-systems Shared Systems Team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@knz @postamar