cockroach icon indicating copy to clipboard operation
cockroach copied to clipboard

clusterversion,changefeedccl: remove ChangefeedIdleness

Open celiala opened this issue 2 years ago • 1 comments

This commit removes the 22.1 ChangefeedIdleness version gate.

Cleanup was done following 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]).

However for just a couple more complicated cases that were referring to <21.2 versions that needed to be replaced when those were deleted, I added a placeholder clusterversion.TODOPre21_2 alias for 21.2. Replacing those calls with this alias shouldn't change their behavior -- it was already always true, since the code today should never run in a <21.2 cluster -- but means we can delete those older versions in the meantime and then the owners of these bits can decide how to update them.

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

Release note: none

celiala avatar Aug 11 '22 01:08 celiala

This change is Reviewable

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

TestLogic_external_connection_privileges is failing all of https://github.com/cockroachdb/cockroach/pull/85937 , https://github.com/cockroachdb/cockroach/pull/85938 , https://github.com/cockroachdb/cockroach/pull/85939, so I'm going to assume that it's not related to changes in this commit.

off_test.runfiles/com_github_cockroachdb_cockroach/pkg/sql/logictest/testdata/logic_test/external_connection_privileges:24: SELECT * FROM system.privileges
        expected:
            root      /externalconn/foo  {ALL}         {}
            testuser  /externalconn/foo  {DROP,USAGE}  {}
        but found (query options: "") :
            testuser  /externalconn/foo  {DROP,USAGE}  {}
            root      /externalconn/foo  {ALL}         {}

celiala avatar Aug 19 '22 02:08 celiala

This tests fails - not sure how to fix - hopefully someone on CDC can help here?

  • pkg/ccl/logictestccl/tests/local-mixed-21.2-22.1/local-mixed-21_2-22_1_test: TestCCLLogic_create_changefeed_mixed_versions

TestCCLLogic_create_changefeed_mixed_versions output:

        expected:
        pq: option end_time is not supported until upgrade to version EnableNewChangefeedOptions or higher is finalized
        got:
        pq: specified end time 1.0000000000 cannot be less than statement time 1660894120070553817.0000000000

celiala avatar Aug 19 '22 13:08 celiala

rebased and force-pushed a version that deletes the local-mixed test. It was only being used to verify we check against these options that no longer exist to be checked.

samiskin avatar Aug 19 '22 19:08 samiskin

TFTR and help with the local-mixed test! 🙌🏼

bors r=samiskin

celiala avatar Aug 19 '22 20:08 celiala

Build failed (retrying...):

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

Build failed (retrying...):

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

Build succeeded:

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