Skip to content

Commit

Permalink
Fix errcheck in ./common/membership/ (#3742)
Browse files Browse the repository at this point in the history
* Fix errcheck in ./common/membership/
  • Loading branch information
MichaelSnowden committed Dec 21, 2022
1 parent 957cfa4 commit 676b744
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
8 changes: 4 additions & 4 deletions common/dynamicconfig/file_based_client_test.go
Expand Up @@ -343,7 +343,7 @@ testGetBoolPropertyKey:
mockLogger.EXPECT().Info("dynamic config changed for the key: testgetboolpropertykey oldValue: { constraints: {} value: false } newValue: { constraints: {} value: true }", gomock.Any())
mockLogger.EXPECT().Info("dynamic config changed for the key: testgetboolpropertykey oldValue: { constraints: {{Namespace:global-samples-namespace}} value: true } newValue: { constraints: {{Namespace:global-samples-namespace}} value: false }", gomock.Any())
mockLogger.EXPECT().Info(gomock.Any())
client.update()
s.NoError(client.update())
s.NoError(err)
close(doneCh)
}
Expand Down Expand Up @@ -393,7 +393,7 @@ history.defaultActivityRetryPolicy:

mockLogger.EXPECT().Info("dynamic config changed for the key: history.defaultactivityretrypolicy oldValue: { constraints: {} value: map[BackoffCoefficient:3 InitialIntervalInSeconds:1 MaximumAttempts:0 MaximumIntervalCoefficient:100] } newValue: { constraints: {} value: map[BackoffCoefficient:2 InitialIntervalInSeconds:3 MaximumAttempts:0 MaximumIntervalCoefficient:100] }", gomock.Any())
mockLogger.EXPECT().Info(gomock.Any())
client.update()
s.NoError(client.update())
s.NoError(err)
close(doneCh)
}
Expand Down Expand Up @@ -446,7 +446,7 @@ testGetIntPropertyKey:
mockLogger.EXPECT().Info("dynamic config changed for the key: testgetfloat64propertykey oldValue: nil newValue: { constraints: {{Namespace:samples-namespace}} value: 22 }", gomock.Any())
mockLogger.EXPECT().Info("dynamic config changed for the key: testgetintpropertykey oldValue: nil newValue: { constraints: {} value: 2000 }", gomock.Any())
mockLogger.EXPECT().Info(gomock.Any())
client.update()
s.NoError(client.update())
s.NoError(err)
close(doneCh)
}
Expand Down Expand Up @@ -503,7 +503,7 @@ testGetFloat64PropertyKey:
reader.EXPECT().ReadFile(gomock.Any()).Return(updatedFileData, nil)

mockLogger.EXPECT().Info(gomock.Any()).Times(1)
client.update()
s.NoError(client.update())
s.NoError(err)
close(doneCh)
}
19 changes: 14 additions & 5 deletions common/membership/grpc_resolver.go
Expand Up @@ -89,7 +89,9 @@ func (m *grpcBuilder) Build(target resolver.Target, cc resolver.ClientConn, opts
r: r,
notifyCh: make(chan *ChangedEvent, 1),
}
resolver.start()
if err := resolver.start(); err != nil {
return nil, err
}
return resolver, nil
}

Expand All @@ -100,14 +102,17 @@ type grpcResolver struct {
wg sync.WaitGroup
}

func (m *grpcResolver) start() {
m.r.AddListener(fmt.Sprintf("%p", m), m.notifyCh)
func (m *grpcResolver) start() error {
if err := m.r.AddListener(fmt.Sprintf("%p", m), m.notifyCh); err != nil {
return err
}
m.wg.Add(1)
go m.listen()

// Try once to get address synchronously. If this fails, it's okay, we'll listen for
// changes and update the resolver later.
m.resolve()
return nil
}

func (m *grpcResolver) listen() {
Expand All @@ -131,7 +136,9 @@ func (m *grpcResolver) resolve() {
Addr: hostInfo.GetAddress(),
})
}
m.cc.UpdateState(resolver.State{Addresses: addresses})
if err := m.cc.UpdateState(resolver.State{Addresses: addresses}); err != nil {
fmt.Printf("error updating state in gRPC resolver: %v", err)
}
}

func (m *grpcResolver) ResolveNow(_ resolver.ResolveNowOptions) {
Expand All @@ -142,7 +149,9 @@ func (m *grpcResolver) ResolveNow(_ resolver.ResolveNowOptions) {
}

func (m *grpcResolver) Close() {
m.r.RemoveListener(fmt.Sprintf("%p", m))
if err := m.r.RemoveListener(fmt.Sprintf("%p", m)); err != nil {
fmt.Printf("error removing listener from gRPC resolver: %v", err)
}
close(m.notifyCh)
m.wg.Wait() // wait until listen() exits
}
5 changes: 3 additions & 2 deletions common/membership/rpMonitor_test.go
Expand Up @@ -28,9 +28,10 @@ import (
"testing"
"time"

"go.temporal.io/server/common/primitives"
"golang.org/x/exp/maps"

"go.temporal.io/server/common/primitives"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
)
Expand Down Expand Up @@ -67,7 +68,7 @@ func (s *RpoSuite) TestRingpopMonitor() {

// Force refresh now and drain the notification channel
resolver, _ := rpm.GetResolver(serviceName)
resolver.(*ringpopServiceResolver).refresh()
s.NoError(resolver.(*ringpopServiceResolver).refresh())
drainChannel(listenCh)

s.T().Log("Killing host 1")
Expand Down

0 comments on commit 676b744

Please sign in to comment.