cockroach icon indicating copy to clipboard operation
cockroach copied to clipboard

clusterversion,kvserver,server,security:remove 22.1 gates

Open celiala opened this issue 2 years ago • 6 comments

This commit removes the following 22.1 version gates:

**Version gates from 8/8 **

  • kvserver: ProbeRequest,
  • security,pgwire: SCRAMAuthentication,
  • server: UnsafeLossOfQuorumRecoveryRangeLog,
  • kvserver: AlterSystemProtectedTimestampAddColumn

8/10 Update: Additionally, these version gates to added to this commit

  • backupccl: EnableProtectedTimestampsForTenant
  • DeleteCommentsWithDroppedIndexes
  • sql/catalog: RemoveIncompatibleDatabasePrivileges
  • kvserver: AddRaftAppliedIndexTermMigration
  • clusterversion: PostAddRaftAppliedIndexTermMigration
  • kvserver: DontProposeWriteTimestampForLeaseTransfers
  • storage: EnablePebbleFormatVersionBlockProperties
  • sql: MVCCIndexBackfiller
  • kvserver: LooselyCoupledRaftLogTruncation
  • sql: RowLevelTTL
  • kvserver: EnableNewStoreRebalancer
  • sql: SuperRegions
  • changefeedccl: EnableNewChangefeedOptions
  • SpanCountTable
  • spanconfig: PreSeedSpanCountTable, SeedSpanCountTable
  • sql: EnableDeclarativeSchemaChanger

The cleanup for these version gates was fairly straight-forward, using the guidance from 21.2 cleanup:

For the most part, if the gates were just simple if !version.IsActive { return x } or something, I just removed the block, and even if it was a little more complicated, like args = [x]; if version { args = append(args, y) }; foo(args) I still tried to mostly inline it such that it looked natural (i.e. remove that append and make it args = [x, y]).

22.1 version gates requiring more nuanced removal will be done in separate PRs.

Partially addresses https://github.com/cockroachdb/cockroach/issues/80663

celiala avatar Aug 08 '22 22:08 celiala

This change is Reviewable

cockroach-teamcity avatar Aug 08 '22 22:08 cockroach-teamcity

@dt @irfansharif - I'm going to remove the version gates in multiple passes, but let me know if version gate removal + bumping minVersion needs to be done more atomically. Thanks!

celiala avatar Aug 09 '22 14:08 celiala

bors r+

celiala avatar Aug 09 '22 19:08 celiala

bors cancel

celiala avatar Aug 09 '22 19:08 celiala

Canceled.

craig[bot] avatar Aug 09 '22 19:08 craig[bot]

I meant to push the additional 8/10 version gate removals to a separate branch, but accidentally pushed to this PR:

Since the 8/10 commits were also straight-forward removals, I will go ahead and keep them in this PR. But this means I'm re-adding @cockroachdb/kv-prs to review the version gates that have been added today.

celiala avatar Aug 11 '22 01:08 celiala

Schema LGTM for:

  • clusterversion: remove DeleteCommentsWithDroppedIndexes
  • clusterversion,sql: remove MVCCIndexBackfiller
  • clusterversion,sql: remove EnableDeclarativeSchemaChanger
  • clusterversion,sql: remove RowLevelTTL
  • clusterversion,sql: remove SuperRegions

postamar avatar Aug 11 '22 13:08 postamar

TFTRs!

bors r+

8/12 Update: These version gate removals were reverted (removed from this PR) to pass tests. To be addressed separately (future PRs):

  • storage: EnablePebbleFormatVersionBlockProperties
  • sql: RowLevelTTL
  • sql: SuperRegions
  • changefeedccl: EnableNewChangefeedOptions

celiala avatar Aug 12 '22 18:08 celiala

Build succeeded:

craig[bot] avatar Aug 12 '22 20:08 craig[bot]