-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix flaky mysqlctl blackbox test #17387
Fix flaky mysqlctl blackbox test #17387
Conversation
This can trigger the race detector: ``` ================== WARNING: DATA RACE Read at 0x00c00031f900 by goroutine 64: vitess.io/vitess/go/vt/mysqlctl/blackbox.AssertLogs() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/blackbox/utils.go:81 +0xb8 vitess.io/vitess/go/vt/mysqlctl/blackbox.TestExecuteBackupFailToWriteFileTwice() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/blackbox/backup_test.go:713 +0x7e8 testing.tRunner() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:1690 +0x184 testing.(*T).Run.gowrap1() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:1743 +0x40 Previous write at 0x00c00031f900 by goroutine 75: vitess.io/vitess/go/vt/mysqlctl/blackbox.TestExecuteBackupFailToWriteFileTwice.NewMemoryLogger.func2() /Users/dirkjan/code/vitessio/vitess/go/vt/logutil/logger.go:225 +0x130 vitess.io/vitess/go/vt/logutil.(*CallbackLogger).InfoDepth() /Users/dirkjan/code/vitessio/vitess/go/vt/logutil/logger.go:139 +0x1a0 vitess.io/vitess/go/vt/logutil.(*CallbackLogger).Infof() /Users/dirkjan/code/vitessio/vitess/go/vt/logutil/logger.go:174 +0x60 vitess.io/vitess/go/vt/logutil.(*MemoryLogger).Infof() <autogenerated>:1 +0x5c vitess.io/vitess/go/vt/mysqlctl.(*backupPipe).ReportProgress() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/builtinbackupengine.go:820 +0xac4 vitess.io/vitess/go/vt/mysqlctl.(*BuiltinBackupEngine).backupFile.gowrap1() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/builtinbackupengine.go:869 +0x90 Goroutine 64 (running) created at: testing.(*T).Run() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:1743 +0x5e0 testing.runTests.func1() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:2168 +0x80 testing.tRunner() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:1690 +0x184 testing.runTests() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:2166 +0x6e0 testing.(*M).Run() /opt/homebrew/Cellar/go/1.23.4/libexec/src/testing/testing.go:2034 +0xb74 main.main() _testmain.go:59 +0x110 Goroutine 75 (finished) created at: vitess.io/vitess/go/vt/mysqlctl.(*BuiltinBackupEngine).backupFile() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/builtinbackupengine.go:869 +0x734 vitess.io/vitess/go/vt/mysqlctl.(*BuiltinBackupEngine).backupFileEntries.func2() /Users/dirkjan/code/vitessio/vitess/go/vt/mysqlctl/builtinbackupengine.go:706 +0x294 golang.org/x/sync/errgroup.(*Group).Go.func1() /Users/dirkjan/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:78 +0x7c ================== ``` Signed-off-by: Dirkjan Bussink <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Dirkjan Bussink <[email protected]>
@@ -246,6 +247,12 @@ func (ml *MemoryLogger) Clear() { | |||
ml.mu.Unlock() | |||
} | |||
|
|||
func (ml *MemoryLogger) LogEvents() []*logutilpb.Event { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but IMO GetEvents()
is a better name. If it's not safe to access Events directly, then it may also make sense to make it private (rename it to events).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17387 +/- ##
==========================================
+ Coverage 67.47% 67.51% +0.04%
==========================================
Files 1581 1581
Lines 253944 253948 +4
==========================================
+ Hits 171353 171464 +111
+ Misses 82591 82484 -107 ☔ View full report in Codecov by Sentry. |
This can trigger the race detector:
Checklist