Skip to content

Commit

Permalink
Keep only the latest value in the health channel (#14087)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
rkapka and prestonvanloon committed Jun 6, 2024
1 parent e037491 commit 8413660
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
9 changes: 7 additions & 2 deletions api/client/beacon/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
6 changes: 0 additions & 6 deletions api/client/beacon/health_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions api/client/beacon/testing/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package testing
import (
"context"
"reflect"
"sync"

"github.com/prysmaticlabs/prysm/v5/api/client/beacon/iface"
"go.uber.org/mock/gomock"
Expand All @@ -16,6 +17,7 @@ var (
type MockHealthClient struct {
ctrl *gomock.Controller
recorder *MockHealthClientMockRecorder
sync.Mutex
}

// MockHealthClientMockRecorder is the mock recorder for MockHealthClient.
Expand All @@ -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)
Expand All @@ -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)
}
Expand Down

0 comments on commit 8413660

Please sign in to comment.