clusterversion: set devBranch to false, version to beta.1

[celiala]

Notes:

• Temporarily developing against master, in order to get tests to pass. However, we plan to merge this PR onto release-24.1 once it is cut (i.e. not intended for master branch)
• This PR will not be merged until we intended to select the v24.1.0-beta.1 release candidate (i.e. when release blockers are cleared, as per https://cockroachlabs.atlassian.net/jira/dashboards/10216)

---

Putting runbook here for the next time:

1. [cockroach_versions.go] set developmentBranch = false
2. search for 1000023 and replace with 23, i.e. 1000023.2, 1000023.1
3. dev gen docs; dev gen bazel
4. dev test pkg/sql/catalog/bootstrap --rewrite -f TestInitialValuesToString

• replace hashes as directed until test passes in
• pkg/sql/catalog/bootstrap/bootstrap_test.go (update value for prevSystemHash)
• pkg/sql/catalog/bootstrap/testdata/testdata (update value for system hash and tenant hash)

Release note: None
Epic: None

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[celiala]

These tests are failing Essential CI:

• github.com/cockroachdb/cockroach/pkg/kv/kvserver: TestRangeMigration
• github.com/cockroachdb/cockroach/pkg/upgrade/upgrades: TestStmtDiagForPlanGistMigration
• github.com/cockroachdb/cockroach/pkg/server: TestSyncAllEngines
• fixing github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc: TestIndexDoesNotStorePrimaryKeyColumnMixedVersion

github.com/cockroachdb/cockroach/pkg/kv/kvserver: TestRangeMigration

=== RUN   TestRangeMigration
    test_log_scope.go:170: test logs captured to: /artifacts/tmp/_tmp/54ddc9df1a6cbe399a248794e040aa3a/logTestRangeMigration1712865623
    test_log_scope.go:81: use -show-logs to present logs inline
F240326 00:26:40.320450 16703 server/init.go:669  [T1,Vsystem,n?] 1  binary version (23.2-upgrading-to-24.1-step-022) less than min supported version (41.0)
F240326 00:26:40.320450 16703 server/init.go:669  [T1,Vsystem,n?] 1 !goroutine 16703 [running]:

github.com/cockroachdb/cockroach/pkg/upgrade/upgrades: TestStmtDiagForPlanGistMigration

Error Trace:  github.com/cockroachdb/cockroach/pkg/upgrade/upgrades/helpers_test.go:114
                              github.com/cockroachdb/cockroach/pkg/upgrade/upgrades_test/pkg/upgrade/upgrades/v23_2_plan_gist_stmt_diagnostics_requests_test.go:75
          Error:        Received unexpected error:
                        relation "statement_diagnostics_requests" (35): constraint ID was missing for constraint "statement_diagnostics_requests_pkey"
                        (1) attached stack trace

github.com/cockroachdb/cockroach/pkg/server: TestSyncAllEngines

  test_server_shim.go:144: cluster virtualization disabled in global scope due to issue: #76378 (expected label: C-bug)
  panic: no valid versions in (23.2-upgrading-to-24.1-step-022, 24.1] [recovered]
  panic: no valid versions in (23.2-upgrading-to-24.1-step-022, 24.1] [recovered]
  panic: no valid versions in (23.2-upgrading-to-24.1-step-022, 24.1] [recovered]
  panic: no valid versions in (23.2-upgrading-to-24.1-step-022, 24.1]
goroutine 19681 [running]:
github.com/cockroachdb/cockroach/pkg/sql/colexecerror.CatchVectorizedRuntimeError.func1()

[celiala]

I can't figure out how to get these tests to pass after setting developmentBranch = false. Perhaps one of you has ideas?

• maybe @RaduBerinde - github.com/cockroachdb/cockroach/pkg/kv/kvserver: TestRangeMigration
• maybe @yuzefovich - github.com/cockroachdb/cockroach/pkg/upgrade/upgrades: TestStmtDiagForPlanGistMigration
• maybe @jbowens - github.com/cockroachdb/cockroach/pkg/server: TestSyncAllEngines

any ideas much appreciated 🙇🏼 !

[RaduBerinde]

I will take a look, probably tomorrow. Just to double-check, this is intended for the release-24.1 branch (once that exists)?

[celiala]

I will take a look, probably tomorrow.

thank you! 🙇🏼

Just to double-check, this is intended for the release-24.1 branch (once that exists)?

Correct: using master temporarily, to get tests to pass. But confirming that:

• PR will be merged onto release-24.1 (so, not master)
• PR will not be merged until we intended to select the v24.1.0-beta.1 release candidate (i.e. when release blockers are cleared, as per https://cockroachlabs.atlassian.net/jira/dashboards/10216)
  • added do-not-merge label so this doesn't get merged onto master

(updated description to clarify intent :) )

[RaduBerinde]

For TestStmtDiagForPlanGistMigration I have no idea why the change affects it but this is the fix:

--- a/pkg/upgrade/upgrades/v23_2_plan_gist_stmt_diagnostics_requests_test.go
+++ b/pkg/upgrade/upgrades/v23_2_plan_gist_stmt_diagnostics_requests_test.go
@@ -144,6 +144,7 @@ func getOldStmtDiagReqsDescriptor() *descpb.TableDescriptor {
                        KeyColumnNames:      []string{"id"},
                        KeyColumnDirections: []catenumpb.IndexColumn_Direction{catenumpb.IndexColumn_ASC},
                        KeyColumnIDs:        []descpb.ColumnID{1},
+                       ConstraintID:        1,
                },
                Indexes: []descpb.IndexDescriptor{
                        {
@@ -158,9 +159,10 @@ func getOldStmtDiagReqsDescriptor() *descpb.TableDescriptor {
                                Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
                        },
                },
-               NextIndexID:    4,
-               Privileges:     catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()),
-               NextMutationID: 1,
-               FormatVersion:  3,
+               NextIndexID:      4,
+               Privileges:       catpb.NewCustomSuperuserPrivilegeDescriptor(privilege.ReadWriteData, username.NodeUserName()),
+               NextMutationID:   1,
+               NextConstraintID: 2,
+               FormatVersion:    3,
        }
 }

[RaduBerinde]

AFAICT TestSyncAllEngines doesn't need to mess with the versions at all, the diff below works. @jbowens am I missing something?

--- a/pkg/server/migration_test.go
+++ b/pkg/server/migration_test.go
@@ -117,16 +117,10 @@ func TestSyncAllEngines(t *testing.T) {
        storeSpec := base.DefaultTestStoreSpec
        storeSpec.StickyVFSID = "sync-all-engines"
        testServerArgs := base.TestServerArgs{
-               Settings: cluster.MakeTestingClusterSettingsWithVersions(
-                       roachpb.Version{Major: 24, Minor: 1},
-                       roachpb.Version{Major: 23, Minor: 1},
-                       false, /* initializeVersion */
-               ),
                StoreSpecs: []base.StoreSpec{storeSpec},
                Knobs: base.TestingKnobs{
                        Server: &TestingKnobs{
-                               BinaryVersionOverride: roachpb.Version{Major: 24, Minor: 1},
-                               StickyVFSRegistry:     vfsRegistry,
+                               StickyVFSRegistry: vfsRegistry,
                        },
                },
        }

[yuzefovich]

TestStmtDiagForPlanGistMigration failure seems similar to #120124 and is probably caused by #120008.

I think the problem is that the table descriptor returned by getOldStmtDiagReqsDescriptor doesn't actually match the actual table descriptor from before "plan gist migration". For example, there is a CHECK constraint on this table (that was there before the plan gist migration), but it's not in the returned table descriptor. If Radu's patch makes the test pass, we probably don't need to look further.

[RaduBerinde]

I think the problem is that the table descriptor returned by getOldStmtDiagReqsDescriptor doesn't actually match the actual table descriptor from before "plan gist migration". For example, there is a CHECK constraint on this table (that was there before the plan gist migration), but it's not in the returned table descriptor. If Radu's patch makes the test pass, we probably don't need to look further.

But why would the test pass on master and start failing just because we changed devBranch=false?

[yuzefovich]

Hm, I don't have any ideas for that.

[celiala]

All tests pass 🎉 -- thanks for the help. I'll re-route this change to release-24.1 after we cut tomorrow.

[celiala]

Now that tests pass and release-24.1 has been cut, closing this in favor of #121290