feat: upgrade to Go 1.26 - #68
Merged
Merged
Conversation
testMW called loadBalancer.predict() to guess which mock the next query would hit, then armed that mock. But predict() advances the round-robin counter, and the resolver's own Resolve() advances it again, so for pools with more than one db the query landed on a different mock than the one armed. Every fuzz config with >1 primary or replica under the round-robin policy failed with sqlmock unmet-expectation errors; random happened to work because its predict() preloads a channel that Resolve() consumes. Replace the guess with a test-only nextIndex() helper that computes the upcoming round-robin index without mutating the counter (and keeps the channel-preload path for random), so the prediction matches the resolver's routing for any cluster size.
bxcodec
force-pushed
the
feat/upgrade-to-go126
branch
from
July 18, 2026 13:08
3e1d7b7 to
dab9aea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bumps the module to Go 1.26 and brings the build tooling with it. The code changes are mechanical; the CI side needed real work to stay green.
Code
go.modnow requiresgo 1.26.0interface{}withanyacross theDB,Conn,Stmt, andTxwrapper interfacesfor i := range nTooling (the part that actually broke)
golangci-lint
1.59.0is built with Go 1.22 and can't read Go 1.26 export data. It failed withunsupported version: 2and threw fake typecheck errors on every file, which would have blocked the lint gate ingo.yml. Fixing it meant moving to golangci-lint v2:1.59.0to2.12.2(built with go1.26.2) inmisc/makefile/tools.Makefile.golangci.yamlto the v2 config format.gofmtandgoimportsmoved into aformattersblock, and the linters that no longer exist in v2 dropped out:exportloopref(pointless under Go 1.22 loopvar), plusgosimpleandstylecheck(both folded intostaticcheck)depguard. It was enabled with no rules, so v2 defaulted to denying the module's owngo.uber.org/multierrimport. It only stayed quiet before because typecheck aborted first. There was no guard intent, so it's gone rather than faked with a deny listG115exclusion for the load balancer counter modulo.x % uint64(n)is bounded bynand always fits, so the overflow warning is a false positive. Matches the existingG404exclusion stylego: "1.26"Workflows
go_cross.yml: matrix moved from1.21.x/1.22.xto1.26.xplusstable. The old versions can't build a module that requires 1.26go_fuzz.yml: dropped the hardcodedgo-version: 1.20.4in favor ofgo-version-file: go.modcheckout@v3to v4,setup-go@v4to v5,cache@v3to v4Verification
Run locally against Go 1.26.5:
go build ./...go vet ./...make lint(golangci-lint 2.12.2)make testgo test -fuzz=FuzzMultiWriteFuzz harness fix
Once CI could actually run the linter and tests,
FuzzMultiWriteturned out to be red, and it had been red onmainfor a while too. Not a Go 1.26 regression, but it was blocking this branch, so it's fixed here.The
testMWhelper guessed which mock the next query would hit by callingloadBalancer.predict(), then armed that mock. Butpredict()advances the round-robin counter, and the resolver advances it again on the real query, so any cluster with more than one DB routed to a different mock than the test armed. Every round-robin config with >1 primary or replica failed with sqlmock unmet-expectation errors. Random happened to work because itspredict()preloads a channel thatResolve()then consumes.The fix keeps production code untouched and replaces the guess with a test-only
nextIndex()helper that computes the upcoming round-robin index without mutating the counter (and keeps the channel-preload path for random), so the prediction matches the resolver's routing for any cluster size.One housekeeping note: CI had auto-committed a crashing fuzz corpus back onto this branch (
ci: fuzz tests updated on). That commit is dropped, since the harness fix makes those inputs pass and fuzzing no longer produces crashes.