Skip to content

Update tests, fix issues - #228

Merged
harshavardhana merged 8 commits into
minio:mainfrom
klauspost:update-tests-go126
Apr 9, 2026
Merged

harshavardhana merged 8 commits into
minio:mainfrom
klauspost:update-tests-go126

Conversation

@klauspost

@klauspost klauspost commented Apr 6, 2026 •

Copy link
Copy Markdown
Contributor
  • Fix certificate reload on Windows (can corrupt heap)
  • Test on more plaforms
  • Pin linter/fumpter in go.mod file
  • Disable windows+symlink tests
  • Move syscall dependent test to platform guarded file
  • Remove toolchain from go.mod - this is an SDK
  • Run tests on alternative platforms.
  • Don't use deprecated 'StringToUTF16Ptr'.
  • Add Go 1.26.x to tester.
  • Min version Go 1.25.x

We really should replace the notifier with something that is active. But not going down that road now.

* Fix certificate reload on Windows.
* Pin linter/fumpter in go.mod file
* Disable windows+symlink tests
* Move syscall dependent test to platform guarded file
* Remove toolchain from go.mod - this is an SDK
* Run tests on alternative platforms.
* Don't use deprecated 'StringToUTF16Ptr'.
* Add Go 1.26.x to tester.
* Min version Go 1.25.x
klauspost added a commit to klauspost/pkg that referenced this pull request Apr 6, 2026
Merge after minio#228 to include arm64 checks.

Before/after...

```
pkg: github.com/minio/pkg/v3/rng
cpu: AMD Ryzen 9 9950X 16-Core Processor
BenchmarkReader
BenchmarkReader/1000-32         	46546988	        25.88 ns/op	38635.30 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1024-32         	70920727	        17.14 ns/op	59755.33 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/16384-32        	 5805674	       204.9 ns/op	79950.02 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1048576-32      	   92539	     14080 ns/op	74470.24 MB/s	       0 B/op	       0 allocs/op

BenchmarkReader/1000-32         	52974752	        22.57 ns/op	44300.70 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1024-32         	100000000	        11.37 ns/op	90096.95 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/16384-32        	14598060	        81.69 ns/op	200552.58 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1048576-32      	  174301	      6384 ns/op	164256.53 MB/s	       0 B/op	       0 allocs/op
```

This comment was marked as resolved.

@klauspost
klauspost requested a review from rraulinio April 7, 2026 12:24
@klauspost
klauspost requested review from aead and marktheunissen April 8, 2026 10:11
@harshavardhana
harshavardhana merged commit e3b26eb into minio:main Apr 9, 2026
10 checks passed
@klauspost
klauspost deleted the update-tests-go126 branch April 9, 2026 15:23
klauspost added a commit that referenced this pull request Apr 9, 2026
* rng: Add AVX2 + NEON asm.

Merge after #228 to include arm64 checks.

Before/after...

```
pkg: github.com/minio/pkg/v3/rng
cpu: AMD Ryzen 9 9950X 16-Core Processor
BenchmarkReader
BenchmarkReader/1000-32         	46546988	        25.88 ns/op	38635.30 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1024-32         	70920727	        17.14 ns/op	59755.33 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/16384-32        	 5805674	       204.9 ns/op	79950.02 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1048576-32      	   92539	     14080 ns/op	74470.24 MB/s	       0 B/op	       0 allocs/op

BenchmarkReader/1000-32         	52974752	        22.57 ns/op	44300.70 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1024-32         	100000000	        11.37 ns/op	90096.95 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/16384-32        	14598060	        81.69 ns/op	200552.58 MB/s	       0 B/op	       0 allocs/op
BenchmarkReader/1048576-32      	  174301	      6384 ns/op	164256.53 MB/s	       0 B/op	       0 allocs/op
```

* Test AVX2 and SSE2 in tests.

* Add missing tag. Test tags.

---------

Co-authored-by: Harshavardhana <harsha@minio.io>
Vonng added a commit to pgsty/silo-pkg that referenced this pull request Aug 2, 2026
Backport of the certs/ portion of minio#228 (e3b26eb).

Manager.AddCertificate() registered two notify.Watch() calls and never
stopped either: if the second one failed the first leaked, and on manager
shutdown both stayed registered for the life of the process.
Certificate.Watch() and watchFile() had the same shape. Route all four
through a new watchDirSafe() that returns a stop function, and call it
from the error path and from ctx.Done().

Note what watchDirSafe() does on Windows: it does not fall back to
polling only when something fails, it replaces filesystem notification
outright on that platform, because rjeczalik/notify casts unsafe pointers
in a way that trips Go's checkptr validation. So on Windows certificate
reload latency becomes up to one symlinkReloadInterval (10s), and the two
call sites that register two watches on one channel now emit two
synthetic events per interval, i.e. two reload passes. Note also that the
trigger for that workaround is checkptr, which is enabled by -race and
-msan, so what it costs is production behaviour on Windows and what it
buys is a test build that does not crash. None of this is verified here
beyond a cross-compile and vet - there is no Windows CI.

loadSystemRoots() separately moves off syscall.StringToUTF16Ptr, which
panics rather than returning an error on an embedded NUL, and eventWrite
picks up notify.Rename so a certificate replaced by rename reads as a
write.

Only the certs/ files are taken. Upstream's go.mod in the same commit
adds golangci-lint as a `tool` directive, which drags ~200 lint
dependencies into the module graph; that part is deliberately skipped.

(cherry picked from commit 8a16d2794b4c3afe3e1b9aa403341f8fbc651b7f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants