dendrite
dendrite copied to clipboard
Switch to Go 1.19 as minimum (attempt 2)
Reasoning
Go 1.19 has been out for over a month now (02/08/2022) and features some changes which are likely to be quite beneficial here (see https://github.com/matrix-org/dendrite/issues/2564). Perhaps most importantly, the release allows one to set a soft memory limit on the runtime which helps a lot in the P2P usecase.
Other noteworthy changes include:
- Performance improvements (especially on RISC-V)
- Some security improvements
- More stdlib atomic types (no need for
go.uber.org/atomicanymore) - Automatic file descriptor limit increase on unix-like systems
Hence, I think it's worth incrementing the minimum supported version to benefit from the above.
As an alternative, I could revert backwards-incompatible changes and just use go 1.19 in CI to benefit from most of this but still allow building with 1.18. That said, I personally think the code removed because of the upgrade warrants incrementing the minimum.
No tests needed beyond what already exists as this doesn't really change the code in any meaningful way.
Changes Summary
- Replace all usage of Go 1.18 with Go 1.19
- Use debian bullseye over debian stretch
- Remove now-obsolete file descriptor limit increase code
- Simplify unix build constraints
- Replace usage of
go.uber.org/atomicwithsync/atomicand move the former into the indirect deps section ingo.mod - Fix a minor typo in log
Side Note
https://github.com/matrix-org/dendrite/blob/af9a204cc0b997bf0b7fd6ea434a7c04157bdf28/syncapi/storage/postgres/filtering.go#L41-L50
I came across this comment while searching for 1.18. I assume the comment means the function should be merged with the duplicate below it using generics? Unfortunately, Go generics don't allow for accessing properties, only methods. Should I remove the comment?
Pull Request Checklist
- [x] I have added added tests for PR or I have justified why this PR doesn't need tests.
- [x] Pull request includes a sign off
Signed-off-by: 0x1a8510f2 <[email protected]>
IMO wait until 1.19 is in debian stable backports. https://packages.debian.org/bullseye-backports/golang
Maybe there's another popular "stable server" distro with a backports equivalent that would be worth considering also.
Actually, is it not already?
https://packages.debian.org/bullseye-backports/golang-1.19-go
By the looks of it, the golang metapackage has not been updated to 1.19 but Debian users can still explicitly install 1.19 from the official repos.
Funnily enough, that's the case on Tumbleweed too which is a rolling distro :P
hmm you are correct
Debian backports' golang is defaulting to 1.19.1 now.
Thanks @bones-was-here. Given that and the lack of objections, I think this is probably safe to merge sometime soon. Note that sytests are broken until https://github.com/matrix-org/sytest/pull/1292 is merged, but I've fixed the other tests.
Side Note
I came across this comment while searching for
1.18. I assume the comment means the function should be merged with the duplicate below it using generics? Unfortunately, Go generics don't allow for accessing properties, only methods. Should I remove the comment?
Ah, yes, that was the idea. The comment can be removed. Added that comment before having actually used generics in Go. From my side this change looks fine.
Thanks for the PR, but for now we've decided against merging this.
The only changes that require actual code modifications are the atomics — the other benefits listed in the comments can all be obtained by just compiling Dendrite today using Go 1.19. I'll commit an update to the dockerfiles to build those using Go 1.19 too.
Go 1.19 is still relatively recent though and may not have tricked down into the distributions that our users care about without extra work, so in the interests of not making life harder for people to run Dendrite, we'll stick with supporting Go 1.18 for the short term.
My server uses an architecture unsupported by gc (the go compiler you all know and love), so I have no other option but to use gccgo.
Gccgo will always lack behind gc in terms of what version of go it supports.
I know building with gccgo is probably not a "supported" usecase, and likely won't ever be for dendrite, but I thought it was worth commenting showing there is at least a single user out there hoping dendrite supports the lowest version of go possible :)
That, admittedly, is not something I've considered. Fair enough!