From 2e322b464c82c123658b4b63df0d03e5f0b3f8ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= <rkapka@wp.pl>
Date: Thu, 6 Jun 2024 20:45:35 +0200
Subject: [PATCH] Keep only the latest value in the health channel (#14087)

* Increase health tracker channel buffer size

* keep only the latest value

* Make health test blocking as a regression test for PR #14807

* Fix new race conditions in the MockHealthClient

---------

Co-authored-by: Preston Van Loon <preston@pvl.dev>
---
 api/client/beacon/health.go       | 9 +++++++--
 api/client/beacon/health_test.go  | 6 ------
 api/client/beacon/testing/mock.go | 6 ++++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/api/client/beacon/health.go b/api/client/beacon/health.go
index c13a04407ee8..9b4f088af1b7 100644
--- 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
index 8dedfc54c95c..a6d3a0627844 100644
--- 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
index 25fe4bcf3aa7..7768d4a7ed41 100644
--- 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)
 }