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

clusterversion: require env var to do poison dev upgrades #87468

Merged
merged 2 commits into from
Sep 8, 2022

Conversation

dt
Copy link
Member

@dt dt commented Sep 6, 2022

Previously the offsetting of all in-development versions ensured that upgrading to one of these would mark the cluster as untrusted, dev-version-only, however the fact we did not offset already released versions meant that one could perform such an upgrade easily, by simply starting a dev binary in a stable release data directory, as upgrades happen by default automatically. This could lead to an inadvertent and irreversible conversion of a cluster to dev versions.

This changes the behavior to default to offsetting all versions, not just the the new ones, which has the effect of also offset the version from which a binary is willing to upgrade. This significantly reduces the risk of inadvertently upgrading a cluster to a dev version, as by default, the dev version will refuse to start in a release-version's data directory.

In some cases however it is useful to start a custom or development build in an existing data directory, e.g. a snapshot collected from production. For these cases, the env var COCKROACH_UPGRADE_TO_DEV_VERSION can be used to only offset the second defined version and above, meaning that the first version, which is typically the minBinaryVersion, is left alone, and that binary thus considers itself backwards compatible with that older release version and will thus be willing to start in / join that existing cluster.

Release note: none.

Release justification: bug fix in new functionality.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

I like it

@dt dt force-pushed the vers-opt-in branch 2 times, most recently from 87f0d0e to 98cfed3 Compare September 7, 2022 01:17
@dt dt requested a review from a team September 7, 2022 01:17
@dt dt requested a review from a team as a code owner September 7, 2022 01:17
@dt dt force-pushed the vers-opt-in branch 5 times, most recently from 55b71dd to 69c0cee Compare September 8, 2022 05:12
dt added 2 commits September 8, 2022 12:22
It consistently times out under race.

Release note: none.
Previously the offsetting of all in-development versions ensured that upgrading to one
of these would mark the cluster as untrusted, dev-version-only, however the fact we did
not offset already released versions meant that one could perform such an upgrade easily,
by simply starting a dev binary in a stable release data directory, as upgrades happen by
default automatically. This could lead to an inadvertent and irreversible conversion of a
cluster to dev versions.

This changes the behavior to default to offsetting _all_ versions, not just the the new ones,
which has the effect of also offset the version _from which_ a binary is willing to upgrade.
This significantly reduces the risk of inadvertently upgrading a cluster to a dev version,
as by default, the dev version will refuse to start in a release-version's data directory.

In some cases however it is useful to start a custom or development build in an existing
data directory, e.g. a snapshot collected from production. For these cases, the env var
COCKROACH_UPGRADE_TO_DEV_VERSION can be used to only offset the second defined version
and above, meaning that the first version, which is typically the minBinaryVersion, is
left alone, and that binary thus considers itself backwards compatible with that older
release version and will thus be willing to start in / join that existing cluster.

Release note: none.

Release justification: bug fix in new functionality.
@dt
Copy link
Member Author

dt commented Sep 8, 2022

rebased on #87568

@ajwerner
Copy link
Contributor

ajwerner commented Sep 8, 2022

Still LGTM

@dt
Copy link
Member Author

dt commented Sep 8, 2022

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 8, 2022

Build succeeded:

@craig craig bot merged commit ce55e1b into cockroachdb:master Sep 8, 2022
@dt dt deleted the vers-opt-in branch September 8, 2022 18:37
srosenberg added a commit to srosenberg/cockroach that referenced this pull request Sep 15, 2022
Recent changes to cockroach_versions (logic) now require setting
COCKROACH_UPGRADE_TO_DEV_VERSION environment variable in order to
allow upgrading a stable release data-dir to a dev. version.
The PR [1] which introduced this env. var. did not correctly backport
the change to all the roachtests which perform this type of upgrade.

Since all roachtests which exercise upgrade paths are intended to
test this upgrade scenario, we set COCKROACH_UPGRADE_TO_DEV_VERSION
by default in roachprod instead of polluting the roachtests with more
config. settings.

Consequently, MakeClusterSettings now returns the default
ClusterSettings which includes COCKROACH_UPGRADE_TO_DEV_VERSION;
generateStartCmd ensures it's passed into cockroach env. via start.sh.

Release note: None

Release justification: bug fix in roachtests.

Resolves:
  cockroachdb#87675
  cockroachdb#87687

[1] cockroachdb#87468
craig bot pushed a commit that referenced this pull request Sep 20, 2022
88005: roachprod: add COCKROACH_UPGRADE_TO_DEV_VERSION to DefaultEnvVars r=srosenberg a=srosenberg

Recent changes to cockroach_versions (logic) now require setting COCKROACH_UPGRADE_TO_DEV_VERSION environment variable in order to allow upgrading a stable release data-dir to a dev. version. The PR [1] which introduced this env. var. did not correctly backport the change to all the roachtests which perform this type of upgrade.

Since all roachtests which exercise upgrade paths are intended to test this upgrade scenario, we set COCKROACH_UPGRADE_TO_DEV_VERSION by default in roachprod instead of polluting the roachtests with more config. settings.

Consequently, MakeClusterSettings now returns the default ClusterSettings which includes COCKROACH_UPGRADE_TO_DEV_VERSION; generateStartCmd ensures it's passed into cockroach env. via start.sh.

Release note: None

Release justification: bug fix in roachtests.

Resolves:
  #87675
  #87687

[1] #87468

88031: admission: enable elastic cpu control by default r=irfansharif a=irfansharif

We want to have this bake through the 23.1 development cycle.

Release note: None (not being backported)

88083: stats: fix buckets for INT2 and INT4 r=yuzefovich a=yuzefovich

Previously, if we needed to create "outer" histogram buckets (which is the case when minimum and maximum values in the column weren't sampled yet they contributed to the distinct count) for INT2 and INT4 types, we would use the values that exceeded the supported range for those types. This could lead to incorrect estimation later on when those "outer" buckets are used during the costing as well as the histograms would need to be manually edited to be injected. This is now fixed by handling these two types separately.

Fixes: #76887.

Release note: None

88167: sql: pg_proc should respect context db for udf r=chengxiong-ruan a=chengxiong-ruan

`pg_proc` table should only return udf in the context db if it's not nil.

Release note: None
Release justification: low risk virtual table improvement.


Co-authored-by: Stan Rosenberg <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
blathers-crl bot pushed a commit that referenced this pull request Sep 20, 2022
Recent changes to cockroach_versions (logic) now require setting
COCKROACH_UPGRADE_TO_DEV_VERSION environment variable in order to
allow upgrading a stable release data-dir to a dev. version.
The PR [1] which introduced this env. var. did not correctly backport
the change to all the roachtests which perform this type of upgrade.

Since all roachtests which exercise upgrade paths are intended to
test this upgrade scenario, we set COCKROACH_UPGRADE_TO_DEV_VERSION
by default in roachprod instead of polluting the roachtests with more
config. settings.

Consequently, MakeClusterSettings now returns the default
ClusterSettings which includes COCKROACH_UPGRADE_TO_DEV_VERSION;
generateStartCmd ensures it's passed into cockroach env. via start.sh.

Release note: None

Release justification: bug fix in roachtests.

Resolves:
  #87675
  #87687

[1] #87468
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants