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,*: remove old versions #65200

Closed
ajwerner opened this issue May 14, 2021 · 7 comments · Fixed by #70268
Closed

clusterversion,*: remove old versions #65200

ajwerner opened this issue May 14, 2021 · 7 comments · Fixed by #70268
Labels
branch-master Failures and bugs on the master branch. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.

Comments

@ajwerner
Copy link
Contributor

Is your feature request related to a problem? Please describe.

In principle we ought to be able to remove old versions from the below file when we move on to the next version. Removing old versions forces removing code-paths which are gated on those versions. That's a good forcing function. This represents a reasonable chunk of work scattered over many teams. This issue tracks the actual removal of the versions.

https://github.com/cockroachdb/cockroach/blob/master/pkg/clusterversion/cockroach_versions.go#L321

@ajwerner ajwerner added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label May 14, 2021
@postamar postamar added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. meta-issue Contains a list of several other issues. labels May 18, 2021
@postamar
Copy link
Contributor

postamar commented May 18, 2021

I'm turning this into a meta issue after an attempt at breaking down the work this entails:

  • AbortSpanBytes
  • AddScheduledJobsTable
  • AlterColumnTypeGeneral
  • AlterSystemJobsAddCreatedByColumns
  • AlterSystemJobsAddSqllivenessColumnsAddNewSystemSqllivenessTable
  • Box2DType
  • CPutInline
  • ChangefeedsSupportPrimaryIndexChanges
  • ClosedTimestampsRaftTransport
  • CreateLoginPrivilege
  • EmptyArraysInInvertedIndexes
  • Enums
  • ForeignKeyRepresentationMigration
  • GeospatialType
  • HBAForNonTLS
  • ImplicitColumnPartitioning
  • LongRunningMigrations
  • MaterializedViews
  • MinPasswordLength
  • MultiRegionFeatures
  • NamespaceTableWithSchemas
  • NamespaceTableWithSchemasMigration
  • NewSchemaChanger
  • NoOriginFKIndexes
  • NodeMembershipStatus
  • NonVotingReplicas
  • PostTruncatedAndRangeAppliedStateMigration
  • PriorReadSummaries
  • ProtectedTsMetaPrivilegesMigration
  • RangefeedLeases
  • replacedPostTruncatedAndRangeAppliedStateMigration
  • replacedTruncatedAndRangeAppliedStateMigration
  • ReplicaVersions
  • SeparatedIntents
  • SequencesRegclass
  • TracingVerbosityIndependentSemantics
  • TruncatedAndRangeAppliedStateMigration
  • UniqueWithoutIndexConstraints
  • UpdateScheduledJobsSchema
  • UserDefinedSchemas
  • VirtualComputedColumns

Some of these are trivial to remove, others not so much.

postamar pushed a commit to postamar/cockroach that referenced this issue May 20, 2021
This commit removes the NamespaceTableWithSchemas and
NamespaceTableWithSchemasMigration cluster version keys
and all the version-gated code pertaining to them.

Notably, this removes all references to system.namespace2 as well as the
deprecated namespace table at descriptor ID 2.

Partially addresses cockroachdb#65200.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue May 25, 2021
This commit removes the NamespaceTableWithSchemas and
NamespaceTableWithSchemasMigration cluster version keys
and all the version-gated code pertaining to them.

Notably, this removes all references to system.namespace2 as well as the
deprecated namespace table at descriptor ID 2.

Partially addresses cockroachdb#65200.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jun 3, 2021
This commit removes the NamespaceTableWithSchemas and
NamespaceTableWithSchemasMigration cluster version keys
and all the version-gated code pertaining to them.

Notably, this removes all references to system.namespace2 as well as the
deprecated namespace table at descriptor ID 2.

Partially addresses cockroachdb#65200.

Release note: None
postamar pushed a commit to postamar/cockroach that referenced this issue Jun 4, 2021
This commit removes the NamespaceTableWithSchemas and
NamespaceTableWithSchemasMigration cluster version keys
and all the version-gated code pertaining to them.

Notably, this removes all references to system.namespace2 as well as the
deprecated namespace table at descriptor ID 2.

Partially addresses cockroachdb#65200.

Release note: None
craig bot pushed a commit that referenced this issue Jun 7, 2021
65340: clusterversion: remove NamespaceTableWithSchemas* r=postamar a=postamar

    clusterversion: remove NamespaceTableWithSchemas*
    
    This commit removes the NamespaceTableWithSchemas and
    NamespaceTableWithSchemasMigration cluster version keys
    and all the version-gated code pertaining to them.
    
    Notably, this removes all references to system.namespace2 as well as the
    deprecated namespace table at descriptor ID 2.
    
    Partially addresses #65200.
    
    Release note: None


    clusterversion: add DeleteDeprecatedNamespaceTableDescriptorMigration
    
    This commit adds a migration which removes all traces of the deprecated
    namespace table descriptor as well as the system.namespace2 table name.
    
    Release note (sql change): Namespace entries may no longer be queried
    via `system.namespace2`.


Co-authored-by: Marius Posta <marius@cockroachlabs.com>
@ajwerner
Copy link
Contributor Author

Okay, broke this down into sub-issues. I'm going to knock off some trivial ones and then pass the buck.

craig bot pushed a commit that referenced this issue Jul 28, 2021
67822: clusterversion,sql: remove old cluster versions corresponding to 20.2 and 21.1 r=postamar a=postamar

This pull request removes all old cluster versions corresponding to 20.2 and 21.1 which fall under the SQL group's responsibility.

Fixes #66546.

See #66546 and #65200 for an exhaustive (and possibly exhausting) list.

68043: importccl: remove protected timestamp from IMPORT INTO r=pbardea a=adityamaru

Previously, IMPORT INTO would lay a protected timestamp
on the table span of the table being imported into.
This would prevent all keys in this span from being GC'ed
during IMPORT execution.

IMPORT INTO however does not allow ingesting keys that
shadow already existing keys. Additionally, during the
IMPORT the table is taken offline and so it does not serve
any online traffic. These two properties of IMPORT INTO
ensure that the old keys (not ingested during IMPORT) will
never become gc'eable during the IMPORT job. Thus, writing
a protected timestamp was redundant.

If the IMPORT job fails we issue a RevertRange to revert
all the newly ingested keys. This RevertRange could
attempt to revert to a time below the GC threshold but
following #61004 this is alright for the same reason
explained above.

Fixes: #67924

IMPORT INTO no longer relies on protected timestamps and
so it can be run within a tenant.

68172: clisqlshell: remove an implicit dependency on os.Stdout r=stevendanna a=knz

This is currently somewhat undertested but that's because the current implementation of `\|` and `\!` are broken anyway. We'll need to rewrite them entirely.

Co-authored-by: Marius Posta <marius@cockroachlabs.com>
Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@postamar postamar added the release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. label Aug 19, 2021
@blathers-crl
Copy link

blathers-crl bot commented Aug 19, 2021

Hi @postamar, please add branch-* labels to identify which branch(es) this release-blocker affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@postamar postamar added the branch-master Failures and bugs on the master branch. label Aug 19, 2021
craig bot pushed a commit that referenced this issue Sep 7, 2021
69664: util: copy go1.16 ParseIP into separate file r=RichardJCai a=RichardJCai

go1.17 is changing how IPs are parsed, leading 0s will not be accepted.
This change ensures that IP addresses with leading 0s are accepted
(which is the case in Postgres)

Release justification: only moved code out of go package to it's own
file.
Release note: None


69868: tracing,clusterversion: remove TracingVerbosityIndependentSemantics r=irfansharif,ajwerner a=knz

Informs #66545
Informs #65200

Release justification: low risk, high benefit changes to existing functionality



69869: clusterversion: add warning to not add new versions to a patch r=ajwerner,dt a=rafiss

This is an important warning to have as the eng team grows.

Release justification: comment-only change
Release note: None

Co-authored-by: richardjcai <caioftherichard@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
craig bot pushed a commit that referenced this issue Sep 8, 2021
69864: clusterversion: remove old versions r=postamar,cameronnunez a=knz

Informs  #66545
Informs #65200

Release justification: low risk, high benefit changes to existing functionality



Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
@irfansharif
Copy link
Contributor

All that's remaining are these demarcation versions:

// v21.1 versions.
//
// Start21_1 demarcates work towards CockroachDB v21.1.
Start21_1
// V21_1 is CockroachDB v21.1. It's used for all v21.1.x patch releases.
V21_1

@postamar
Copy link
Contributor

I'm curious as to why these can't also be removed. I see that they're being used but I don't understand what makes these versions special but I'm guessing there's a deeper reason. If someone were to add a few comments to explain this I'd be OK with closing this issue, for whatever that's worth.

@irfansharif
Copy link
Contributor

Start21_1 is not special, we can remove it (#70268). The conditionals where it's being checked cannot evaluate to true any more. As for V21_1, it should be removed as part of #69828; left a TODO for Celia.

craig bot pushed a commit that referenced this issue Sep 15, 2021
68282: cli,log: allow use of `debug merge-logs` on older logs r=knz a=ajwerner

Fixes [#68278](#68278).

Log parsers require the flag `--format` when parsing older logs (because 
they do not contain format specification). With this patch, this is no longer 
a requirement as the log format is now inferred based on the structure of 
the log if no log format specification exists.

Release justification: bug fix

Release note (bug fix): The debug merge-logs command no longer returns an error 
when the log decoder attempts to parse older logs.

69903: importccl: add support for IMPORT INTO RBR table r=arulajmani,ajstorm,dt a=adityamaru

This change overrides the `default_to_database_primary_region`
and `gateway_region` to always return the primary region of the
database of the table being imported into. This allows for
IMPORT INTO an RBR table.

To ensure that the import is idempotent across resumptions, we cache
the primary region of the database being imported into, during planning.
This information is store in the job details and flow spec to be used
when evaluating the relevant default expr/computed column.

Since IMPORT is a job, it does not have an associated session data
and so it cannot rely on the planners' implementation of the regional
operator. This change also implements the relevant methods in the
`importRegionOperator` to allow resolution of the primary region
of the database being imported into.

Fixes: #69616

Release note (sql change): IMPORT INTO regional by row tables
is supported.

Release justification: fixes for high-priority or high-severity bugs in existing functionality

70150: server: fix TestAdminAPIJobs failure r=knz a=adityamaru

This change sorts the expected job IDs before ensuring
that they are equal.

Fixes: #69401

Release note: None

70226: changefeedccl: updated retryable error warning message r=wongio123 a=wongio123

Retryable error warning message contained the word "error"
Confusing to users because warning message had the word "error" in it
Prefaced warning message with "WARNING"

Release note (enterprise change): updated retyable error warning message to begin with "WARNING"

Closes #69677 

70229: [CRDB-9016] ui: fix drag to zoom on custom charts r=Santamaura a=Santamaura

This PR addresses the issue where a user creates a custom chart and selects an area to zoom into which leaves the grey highlight after the graph zooms in. This was due to the history prop not being passed into the linegraph component and caused an error to throw when updating the url params. This was resolved by passing in the history to propagate to the
linegraph component.

Release note (ui change): fix drag to zoom on custom charts

https://user-images.githubusercontent.com/17861665/133342585-d7b37e9b-7eb8-4a48-b2c5-814fed62556a.mp4



70262: ui: add column selector to transation page r=maryliag a=maryliag

Add column selector to Transaction Page

Fixes #70148

<img width="414" alt="Screen Shot 2021-09-15 at 11 28 56 AM" src="https://user-images.githubusercontent.com/1017486/133463202-7ed7ac3a-9614-4101-ad76-8f431defe688.png">


Release justification: Category 4
Release note (ui change): Add column selector to transaction page

70268: clusterversion: remove Start21_1 (no longer applicable) r=irfansharif a=irfansharif

Fixes #65200. The last remaining 21.1 version (V21_1) can be removed as
part of #69828.

Release note: None

Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Alex Wong <alex.wong@cockroachlabs.com>
Co-authored-by: Santamaura <santamaura@cockroachlabs.com>
Co-authored-by: Marylia Gutierrez <marylia@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
@craig craig bot closed this as completed in 887e84a Sep 15, 2021
@postamar
Copy link
Contributor

Thanks for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants