[release-20.0] Fix potential deadlock in health streamer (#17261) (#1…

…7269) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
vitessio · Nov 21, 2024 · 5635fc2 · 5635fc2
1 parent ae9cc19
commit 5635fc2
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 2 deletions.
diff --git a/go/vt/vttablet/tabletserver/health_streamer.go b/go/vt/vttablet/tabletserver/health_streamer.go
@@ -317,8 +317,10 @@ func (hs *healthStreamer) SetUnhealthyThreshold(v time.Duration) {
 // so it can read and write to the MySQL instance for schema-tracking.
 func (hs *healthStreamer) MakePrimary(serving bool) {
 	hs.fieldsMu.Lock()
-	defer hs.fieldsMu.Unlock()
 	hs.isServingPrimary = serving
+	// We let go of the lock here because we don't want to hold the lock when calling RegisterNotifier.
+	// If we keep holding the lock, there is a potential deadlock that can happen.
+	hs.fieldsMu.Unlock()
 	// We register for notifications from the schema Engine only when schema tracking is enabled,
 	// and we are going to a serving primary state.
 	if serving && hs.signalWhenSchemaChange {

diff --git a/go/vt/vttablet/tabletserver/health_streamer_test.go b/go/vt/vttablet/tabletserver/health_streamer_test.go
@@ -589,13 +589,14 @@ func TestDeadlockBwCloseAndReload(t *testing.T) {
 
 	wg := sync.WaitGroup{}
 	wg.Add(2)
-	// Try running Close and reload in parallel multiple times.
+	// Try running Close & MakePrimary and reload in parallel multiple times.
 	// This reproduces the deadlock quite readily.
 	go func() {
 		defer wg.Done()
 		for i := 0; i < 100; i++ {
 			hs.Close()
 			hs.Open()
+			hs.MakePrimary(true)
 		}
 	}()