Keep only the latest value in the health channel (prysmaticlabs#14087)

* Increase health tracker channel buffer size * keep only the latest value * Make health test blocking as a regression test for PR prysmaticlabs#14807 * Fix new race conditions in the MockHealthClient --------- Co-authored-by: Preston Van Loon <[email protected]>
gobitfly · Jun 6, 2024 · 8413660 · 8413660
1 parent e037491
commit 8413660
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 8 deletions.
diff --git a/api/client/beacon/health.go b/api/client/beacon/health.go
@@ -48,8 +48,13 @@ func (n *NodeHealthTracker) CheckHealth(ctx context.Context) bool {
 	if isStatusChanged {
 		// Update the health status
 		n.isHealthy = &newStatus
-		// Send the new status to the health channel
-		n.healthChan <- newStatus
+		// Send the new status to the health channel, potentially overwriting the existing value
+		select {
+		case <-n.healthChan:
+			n.healthChan <- newStatus
+		default:
+			n.healthChan <- newStatus
+		}
 	}
 	return newStatus
 }
diff --git a/api/client/beacon/health_test.go b/api/client/beacon/health_test.go
@@ -87,12 +87,6 @@ func TestNodeHealth_Concurrency(t *testing.T) {
 	// Number of goroutines to spawn for both reading and writing
 	numGoroutines := 6
 
-	go func() {
-		for range n.HealthUpdates() {
-			// Consume values to avoid blocking on channel send.
-		}
-	}()
-
 	wg.Add(numGoroutines * 2) // for readers and writers
 
 	// Concurrently update health status

diff --git a/api/client/beacon/testing/mock.go b/api/client/beacon/testing/mock.go
@@ -3,6 +3,7 @@ package testing
 import (
 	"context"
 	"reflect"
+	"sync"
 
 	"github.com/prysmaticlabs/prysm/v5/api/client/beacon/iface"
 	"go.uber.org/mock/gomock"
@@ -16,6 +17,7 @@ var (
 type MockHealthClient struct {
 	ctrl     *gomock.Controller
 	recorder *MockHealthClientMockRecorder
+	sync.Mutex
 }
 
 // MockHealthClientMockRecorder is the mock recorder for MockHealthClient.
@@ -25,6 +27,8 @@ type MockHealthClientMockRecorder struct {
 
 // IsHealthy mocks base method.
 func (m *MockHealthClient) IsHealthy(arg0 context.Context) bool {
+	m.Lock()
+	defer m.Unlock()
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "IsHealthy", arg0)
 	ret0, ok := ret[0].(bool)
@@ -41,6 +45,8 @@ func (m *MockHealthClient) EXPECT() *MockHealthClientMockRecorder {
 
 // IsHealthy indicates an expected call of IsHealthy.
 func (mr *MockHealthClientMockRecorder) IsHealthy(arg0 any) *gomock.Call {
+	mr.mock.Lock()
+	defer mr.mock.Unlock()
 	mr.mock.ctrl.T.Helper()
 	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsHealthy", reflect.TypeOf((*MockHealthClient)(nil).IsHealthy), arg0)
 }