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

Build current and next Go version #1163

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

timvaillancourt
Copy link
Collaborator

@timvaillancourt timvaillancourt commented Aug 11, 2022

Description

This PR updates to Golang 1.20 and concurrently tests that gh-ost builds on the following golang versions:

  1. Current - the current repo Golang version (now 1.20)
  2. Next - the upcoming Golang version (now 1.21)

Only the "current" job uploads the binary artifact due to an expression

Example in Actions (notice upload is skipped because 1.21 is not the current version):
TBD

Also, many Actions were updated to recent major versions without issue. TIL action/setup-go supports go-version-file: go.mod to read the version from go.mod vs duplicating 🎉

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

@timvaillancourt timvaillancourt requested review from dm-2 and rashiq August 11, 2022 20:27
@timvaillancourt timvaillancourt marked this pull request as ready for review August 11, 2022 20:27
@timvaillancourt timvaillancourt force-pushed the ci-build-latest-go branch 2 times, most recently from 8be1b63 to 197aa6d Compare August 11, 2022 20:51
@dm-2
Copy link
Contributor

dm-2 commented Aug 12, 2022

What do you think about just supporting current and next? I can't imagine that we'd want to spend much (any?) time on fixing issues specific to min and previous versions when we've already made the jump to current, and it might prevent us from using newer golang features - curious to hear your thoughts!

@timvaillancourt
Copy link
Collaborator Author

timvaillancourt commented Aug 12, 2022

What do you think about just supporting current and next? I can't imagine that we'd want to spend much (any?) time on fixing issues specific to min and previous versions when we've already made the jump to current, and it might prevent us from using newer golang features - curious to hear your thoughts!

@dm-2 that's a good point I didn't think of - if someone had a problem on older golang we'd just tell them to use "current". I'll make that change 👍

I've been listing the minimum Golang version in README.md as 2 major-versions behind "current", but given this discussion I think I'll narrow that gap to 1 major-version behind "current" as well EDIT: nevermind, skipping that change for now

@timvaillancourt timvaillancourt changed the title Build min, previous, current and next Go version Build current and next Go version Aug 18, 2022
@timvaillancourt timvaillancourt added this to the v1.1.6 milestone Aug 18, 2022
dm-2
dm-2 previously approved these changes Sep 6, 2022
@timvaillancourt
Copy link
Collaborator Author

CI is exploding at:

Testing: timestamp-datetime.....
ERROR timestamp-datetime execution failure. cat /tmp/gh-ost-test.log:
2022-09-06 14:05:50 INFO starting gh-ost 788e28ac7b449206a3d575ba90a1137440591c6b
2022-09-06 14:05:50 INFO Migrating `test`.`gh_ost_test`
2022-09-06 14:05:50 INFO inspector connection validated on 127.0.0.1:20618
2022-09-06 14:05:50 INFO User has SUPER, REPLICATION SLAVE privileges, and has ALL privileges on `test`.*
2022-09-06 14:05:50 INFO binary logs validated on 127.0.0.1:20618
2022-09-06 14:05:50 INFO Inspector initiated on fv-az239-850:20618, version 8.0.16
2022-09-06 14:05:50 ERROR Cannot find table `test`.`gh_ost_test`!
2022-09-06 14:05:50 INFO Tearing down inspector
2022-09-06 14:05:50 FATAL 2022-09-06 14:05:50 ERROR Cannot find table `test`.`gh_ost_test`!
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.18.5/x64/src/runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
	/opt/hostedtoolcache/go/1.18.5/x64/src/runtime/debug/stack.go:16 +0x19
github.com/openark/golib/log.logErrorEntry(0x0?, {0x8303a0, 0xc00020a5b0})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/vendor/github.com/openark/golib/log/log.go:188 +0x98
github.com/openark/golib/log.Fatale({0x8303a0, 0xc00020a5b0})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/vendor/github.com/openark/golib/log/log.go:265 +0x2b
github.com/github/gh-ost/go/base.(*simpleLogger).Fatale(0xc0000a4000, {0x8303a0?, 0xc00020a5b0?})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/go/base/default_logger.go:63 +0x29
main.main()
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/go/cmd/gh-ost/main.go:306 +0x2b70
+ FAIL

I wonder if this is a golang 1.18 problem 🤔

@dm-2
Copy link
Contributor

dm-2 commented Sep 6, 2022

CI is exploding at:

Testing: timestamp-datetime.....
ERROR timestamp-datetime execution failure. cat /tmp/gh-ost-test.log:
2022-09-06 14:05:50 INFO starting gh-ost 788e28ac7b449206a3d575ba90a1137440591c6b
2022-09-06 14:05:50 INFO Migrating `test`.`gh_ost_test`
2022-09-06 14:05:50 INFO inspector connection validated on 127.0.0.1:20618
2022-09-06 14:05:50 INFO User has SUPER, REPLICATION SLAVE privileges, and has ALL privileges on `test`.*
2022-09-06 14:05:50 INFO binary logs validated on 127.0.0.1:20618
2022-09-06 14:05:50 INFO Inspector initiated on fv-az239-850:20618, version 8.0.16
2022-09-06 14:05:50 ERROR Cannot find table `test`.`gh_ost_test`!
2022-09-06 14:05:50 INFO Tearing down inspector
2022-09-06 14:05:50 FATAL 2022-09-06 14:05:50 ERROR Cannot find table `test`.`gh_ost_test`!
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/hostedtoolcache/go/1.18.5/x64/src/runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
	/opt/hostedtoolcache/go/1.18.5/x64/src/runtime/debug/stack.go:16 +0x19
github.com/openark/golib/log.logErrorEntry(0x0?, {0x8303a0, 0xc00020a5b0})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/vendor/github.com/openark/golib/log/log.go:188 +0x98
github.com/openark/golib/log.Fatale({0x8303a0, 0xc00020a5b0})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/vendor/github.com/openark/golib/log/log.go:265 +0x2b
github.com/github/gh-ost/go/base.(*simpleLogger).Fatale(0xc0000a4000, {0x8303a0?, 0xc00020a5b0?})
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/go/base/default_logger.go:63 +0x29
main.main()
	/home/runner/work/gh-ost/gh-ost/.gopath/src/github.com/github/gh-ost/go/cmd/gh-ost/main.go:306 +0x2b70
+ FAIL

I wonder if this is a golang 1.18 problem 🤔

I've noticed this error on a couple of other PRs today, re-running CI was successful - I haven't dug into why!

@timvaillancourt
Copy link
Collaborator Author

timvaillancourt commented Sep 6, 2022

I've noticed this error on a couple of other PRs today, re-running CI was successful - I haven't dug into why!

Oh ok, in that case it's not a go 1.18 problem as this PR isn't merged 🤔

EDIT: misread the output. We print-out a golang stack on test errors, I misread this as a panic

@timvaillancourt timvaillancourt modified the milestones: v1.1.6, v1.1.7 Dec 7, 2023
Signed-off-by: Tim Vaillancourt <[email protected]>
@timvaillancourt timvaillancourt removed this from the v1.1.7 milestone Jan 6, 2024
@timvaillancourt timvaillancourt marked this pull request as draft January 6, 2024 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants