From 53ec6bb200c0178371cf7a6f4d30c12b414fb109 Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Thu, 21 Nov 2024 12:07:34 +0530 Subject: [PATCH] Fix potential deadlock in health streamer (#17261) Signed-off-by: Manan Gupta --- go/vt/vttablet/tabletserver/health_streamer.go | 4 +++- go/vt/vttablet/tabletserver/health_streamer_test.go | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/go/vt/vttablet/tabletserver/health_streamer.go b/go/vt/vttablet/tabletserver/health_streamer.go index ffb42df747f..87a5235c2b2 100644 --- 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 index 3a53b1c642d..33b22be550d 100644 --- a/go/vt/vttablet/tabletserver/health_streamer_test.go +++ b/go/vt/vttablet/tabletserver/health_streamer_test.go @@ -587,13 +587,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) } }()