*: upgrade to go 1.20.8 #109773

healthy-pod · 2023-08-30T21:01:26Z

Release note (build change): upgrade to go 1.20.8
Epic: none

cockroach-teamcity · 2023-08-30T21:01:34Z

This change is

rharding6373

for sql queries

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @lidorcarmel, @renatolabs, and @smg260)

* [ ] Adjust the Pebble tests to run in new version. * [ ] Adjust version in Docker image ([source](./builder/Dockerfile)). * [ ] Adjust version in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh)) * [ ] Rebuild and push the Docker image (following [Basic Process](#basic-process)) * [ ] Update `build/teamcity/internal/release/build-and-publish-patched-go/impl.sh` with the new version and adjust SHA256 sums as necessary. * [ ] Adjust `GO_VERSION` and `GO_FIPS_COMMIT` for the FIPS Go toolchain ([source](./teamcity/internal/release/build-and-publish-patched-go/impl-fips.sh)). * [ ] Run the `Internal / Cockroach / Build / Toolchains / Publish Patched Go for Mac` build configuration in TeamCity with your latest version of the script above. Note the job depends on another job `Build and Publish Patched Go`. That job prints out the SHA256 of all tarballs, which you will need to copy-paste into `WORKSPACE` (see below). `Publish Patched Go for Mac` is an extra step that publishes the *signed* `go` binaries for macOS. That job also prints out the SHA256 of the Mac tarballs in particular. * [ ] Adjust `--@io_bazel_rules_go//go/toolchain:sdk_version` in [.bazelrc](../.bazelrc). * [ ] Bump the version in `WORKSPACE` under `go_download_sdk`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases). Also edit the filenames listed in `sdks` and update all the hashes to match what you built in the step above. * [ ] Bump the version in `WORKSPACE` under `go_download_sdk` for the FIPS version of Go (`go_sdk_fips`). * [ ] Run `./dev generate bazel` to refresh `distdir_files.bzl`, then `bazel fetch @distdir//:archives` to ensure you've updated all hashes to the correct value. * [ ] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)). * [ ] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release. * [ ] Bump the go version in `go.mod`. * [ ] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)). * [ ] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`). * [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects. * [ ] Ask the Developer Infrastructure team to deploy new TeamCity agent images according to [packer/README.md](./packer/README.md) Release note (build change): upgrade to go 1.20.8 Epic: none

nvanbenschoten

for kv. Thanks for pushing on this @healthy-pod.

Here are a pair of issues for improvements that we can make once this upgrade lands:

Also, what are our plans with Go 1.21? It's been out for exactly a month and we have about another month to stability for the v23.2 release. Should we immediately follow up this upgrade with the next one?

Reviewed 19 of 29 files at r1, 7 of 9 files at r2, 21 of 21 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @healthy-pod, @irfansharif, @lidorcarmel, @renatolabs, and @smg260)

pkg/util/schedulerlatency/testdata/histogram_buckets line 105 at r3 (raw file):

----
bucket[  0] width=0s                 boundary=[-Inf, 0s)
bucket[  1] width=64ns               boundary=[0s, 64ns)

Do we know which change to the go stdlib caused this reduction in histogram buckets in the runtime/metrics package? @irfansharif are you aware of this change?

healthy-pod · 2023-09-08T16:19:48Z

Do we know which change to the go stdlib caused this reduction in histogram buckets in the runtime/metrics package?

related comment: #97260 (review)

healthy-pod · 2023-09-08T18:05:02Z

bors r+

craig · 2023-09-08T18:51:45Z

Build succeeded:

Bazel Essential CI (Cockroach)

healthy-pod · 2023-09-08T19:02:02Z

Also, what are our plans with Go 1.21? It's been out for exactly a month and we have about another month to stability for the v23.2 release. Should we immediately follow up this upgrade with the next one?

@nvanbenschoten We were planning to upgrade to Go 1.21 directly but Go FIPS 1.21 is not out yet so upgrading non-fips to 1.21 would leave us with two different major Go versions on the same branch which can cause numerous issues. We decided to keep non-fips Go major version and the fips one in sync on the same branch at all times. Once Go FIPS 1.21 is out we can upgrade to Go 1.21 immediately.

nvanbenschoten · 2023-09-08T19:04:20Z

Once Go FIPS 1.21 is out we can upgrade to Go 1.21 immediately.

golang-fips/go#114 merged yesterday. Is that what we were waiting for?

healthy-pod · 2023-09-08T19:11:17Z

golang-fips/go#114 merged yesterday. Is that what we were waiting for?

Nice, we will still wait until a go1.21-fips-release release branch is cut.

By running under `-race`, the go command defines the `race` build tag for us [1]. Previously, we defined it under `TAGS` to let the issue poster know that this is a failure under `race` and indicate that in the issue. At the time, defining the tag twice didn't cause issues but after cockroachdb#109773, it led to build failures [2]. To reproduce locally: ``` bazel test -s --config=race pkg/util/ctxgroup:all --test_env=GOTRACEBACK=all --define gotags=bazel,gss,race ``` As a follow-up, we should find another way to let the issue poster know that a failure was running under `race`. [1] https://go.dev/doc/articles/race_detector#Excluding_Tests [2] cockroachdb#109994 (comment) Epic: none Release note: None

108175: kvserver: unskip lease preferences during outage r=andrewbaptist a=kvoli Previously, `TestLeasePreferenceDuringOutage` would force replication queue processing of the test range, then assert that the range up-replicated and lease transferred to a preferred locality. This test was skipped, and two of the assumptions it relied on to pass were no longer true. After #85219, the replicate queue no longer re-processes replicas. Instead, the queue requeues replicas after processing, at the appropriate priority. This broke the test due to the replicate queue being disabled, making the re-queue a no-op. After #94023, the replicate queue no longer looked for lease transfers, after processing a replication action. Combined with #85219, the queue would now be guaranteed to not process both up-replication and lease transfers from a single enqueue. Update the test to not require a manual process, instead using a queue range filter, which allows tests which disable automatic replication, to still process filtered ranges via the various replica queues. Also, ensure that the non-stopped stores are considered live targets, after simulating an outage (bumping manual clocks, stopping servers) -- so that the expected up-replication, then lease transfer can proceed. Fixes: #88769 Release note: None 109432: cluster-ui: handle partial response errors on the database details page r=THardy98 a=THardy98 Part of: #102386 **Demos** (Note: these demos show this same logic applied to both the databases and database table pages as well): DB-Console - https://www.loom.com/share/5108dd655ad342f28323e72eaf68219c - https://www.loom.com/share/1973383dacd7494a84e10bf39e5b85a3 This change applies the same error handling ideas from #109245 to the database details page, enabling non-admin users to use the database details page and providing better transparency to data fetching issues. Errors encountered while fetching table details can be viewed via the tooltip provided by the `Caution` icon at the table's name. `unavailable` cells also provide a tooltip that displays the error impacting that exact cell. Release note (ui change): Non-admin users are able to use the database details page. 110292: c2c: use seperate spanConfigEventStreamSpec in the span config event stream r=stevendanna a=msbutler Previously, the spanConfigEventStream used a streamPartitionSpec, which contained a bunch of fields unecessary for span config streaming. This patch creates a new spanConfigEventStreamSpec which contains the fields only necessary for span config event streaming. Informs #109059 Release note: None 110309: teamcity-trigger: ensure that `race` tag is only passed once r=healthy-pod a=healthy-pod By running under `-race`, the go command defines the `race` build tag for us [1]. Previously, we defined it under `TAGS` to let the issue poster know that this is a failure under `race` and indicate that in the issue. At the time, defining the tag twice didn't cause issues but after #109773, it led to build failures [2]. To reproduce locally: ``` bazel test -s --config=race pkg/util/ctxgroup:all --test_env=GOTRACEBACK=all --define gotags=bazel,gss,race ``` As a follow-up, we should find another way to let the issue poster know that a failure was running under `race`. [1] https://go.dev/doc/articles/race_detector#Excluding_Tests [2] #109994 (comment) Epic: none Release note: None Co-authored-by: Austen McClernon <[email protected]> Co-authored-by: Thomas Hardy <[email protected]> Co-authored-by: Michael Butler <[email protected]> Co-authored-by: healthy-pod <[email protected]>

`TestMakePriority` was skipped in cockroachdb#110354 due to flakes. The test began to flake after upgrading to `go1.20` in cockroachdb#109773, as the global rand was seeded differently. Bump the number of trials from 100k to 750k in order to collect more samples of the underlying distribution, which deflakes the test. This doubles the average test time, from 200ms to 400ms on a GCP C2 instance. Passes 50k runs: ``` dev test pkg/roachpb -f TestMakePriority -v --stress --count=50000 ... //pkg/roachpb:roachpb_test PASSED in 0.9s Stats over 50000 runs: max = 0.9s, min = 0.3s, avg = 0.4s, dev = 0.0s ``` Fixes: cockroachdb#110303 Release note: None

107577: lint: add new deferunlockcheck pass r=Santamaura a=Santamaura This commit adds a new pass which checks for `...Unlock()` expressions without `defer` which could result in a lock being held indefinitely. Resolves #105366 Release note: None 110376: roachtest: fix logic for exact replication r=arulajmani,kvoli a=andrewbaptist Previously the check for exact replication was incorrectly counting the number of replicas that were NOT at the desired replication count. This fortunately worked becase it was a no-op in the only place that used it since it was run immediately when all replicas had only a single replica. This correctly fixes the check. Epic: none Fixes: #109905 Fixes: #109906 Release note: None 110379: roachpb: deflake and unskip test make priority r=arulajmani,nvanbenschoten a=kvoli `TestMakePriority` was skipped in #110354 due to flakes. The test began to flake after upgrading to `go1.20` in #109773, as the global rand was seeded differently. Bump the number of trials from 100k to 750k in order to collect more samples of the underlying distribution, which deflakes the test. This doubles the average test time, from 200ms to 400ms on a GCP C2 instance. Passes 50k runs: ``` dev test pkg/roachpb -f TestMakePriority -v --stress --count=50000 ... //pkg/roachpb:roachpb_test PASSED in 0.9s Stats over 50000 runs: max = 0.9s, min = 0.3s, avg = 0.4s, dev = 0.0s ``` Fixes: #110303 Release note: None Co-authored-by: Alex Santamaura <[email protected]> Co-authored-by: Andrew Baptist <[email protected]> Co-authored-by: Austen McClernon <[email protected]>

rafiss · 2023-09-12T15:24:46Z

This PR has changed the uuid.FastV4 function to be backed by crypto/rand instead of math/rand. Was that intentional? In #30236 (comment) @nvanbenschoten found that the usage of crypto/rand would block on waiting for more entropy, and could affect performance in some workloads.

healthy-pod requested a review from a team as a code owner August 30, 2023 21:01

healthy-pod force-pushed the go120upgrade branch 15 times, most recently from 3476a25 to 29c6b16 Compare August 31, 2023 05:15

healthy-pod requested review from a team as code owners August 31, 2023 05:15

healthy-pod requested review from rharding6373, smg260, renatolabs and lidorcarmel and removed request for a team August 31, 2023 05:15

rharding6373 reviewed Aug 31, 2023

View reviewed changes

healthy-pod force-pushed the go120upgrade branch from 29c6b16 to 273d457 Compare August 31, 2023 19:16

healthy-pod force-pushed the go120upgrade branch 4 times, most recently from 615a6a7 to 0d15e12 Compare September 7, 2023 18:02

healthy-pod force-pushed the go120upgrade branch from 0d15e12 to 78fee99 Compare September 7, 2023 18:50

rail approved these changes Sep 8, 2023

View reviewed changes

This was referenced Sep 8, 2023

*: use new slice to array conversion syntax #110261

Closed

build: use profile-guided optimization #110262

Open

nvanbenschoten approved these changes Sep 8, 2023

View reviewed changes

craig bot merged commit 397dc0b into cockroachdb:master Sep 8, 2023
2 of 3 checks passed

healthy-pod deleted the go120upgrade branch September 8, 2023 18:55

cockroach-teamcity mentioned this pull request Sep 9, 2023

PR #109773 - *: upgrade to go 1.20.8 cockroachdb/docs#17850

Open

healthy-pod mentioned this pull request Sep 9, 2023

teamcity-trigger: ensure that race tag is only passed once #110309

Merged

kvoli mentioned this pull request Sep 11, 2023

roachpb: deflake and unskip test make priority #110379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: upgrade to go 1.20.8 #109773

*: upgrade to go 1.20.8 #109773

healthy-pod commented Aug 30, 2023 •

edited

Loading

cockroach-teamcity commented Aug 30, 2023

rharding6373 left a comment

nvanbenschoten left a comment

healthy-pod commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

craig bot commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

nvanbenschoten commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

rafiss commented Sep 12, 2023

*: upgrade to go 1.20.8 #109773

*: upgrade to go 1.20.8 #109773

Conversation

healthy-pod commented Aug 30, 2023 • edited Loading

cockroach-teamcity commented Aug 30, 2023

rharding6373 left a comment

Choose a reason for hiding this comment

nvanbenschoten left a comment

Choose a reason for hiding this comment

healthy-pod commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

craig bot commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

nvanbenschoten commented Sep 8, 2023

healthy-pod commented Sep 8, 2023

rafiss commented Sep 12, 2023

healthy-pod commented Aug 30, 2023 •

edited

Loading