Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove common.Daemon from Monitor interface #4084

Merged
merged 1 commit into from
Mar 28, 2023
Merged

Remove common.Daemon from Monitor interface #4084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions common/membership/interfaces.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (

"go.temporal.io/api/serviceerror"

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

Expand All @@ -59,8 +58,6 @@ type (
// Monitor provides membership information for all temporal services.
// It can be used to query which member host of a service is responsible for serving a given key.
Monitor interface {
common.Daemon

WhoAmI() (HostInfo, error)
// EvictSelf evicts this member from the membership ring. After this method is
// called, other members will discover that this node is no longer part of the
Expand Down
24 changes: 0 additions & 24 deletions common/membership/interfaces_mock.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 9 additions & 13 deletions common/membership/ringpop/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ type factory struct {
servicePortMap map[primitives.ServiceName]int
logger log.Logger

membershipMonitor membership.Monitor
metadataManager persistence.ClusterMetadataManager
rpcConfig *config.RPC
tlsFactory encryption.TLSConfigProvider
dc *dynamicconfig.Collection
monitor *monitor
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this to the struct type instead of keep the interface type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have access to the Start/Stop methods which are no longer in the interface

metadataManager persistence.ClusterMetadataManager
rpcConfig *config.RPC
tlsFactory encryption.TLSConfigProvider
dc *dynamicconfig.Collection

chOnce sync.Once
monOnce sync.Once
Expand Down Expand Up @@ -100,12 +100,8 @@ func newFactory(
}, nil
}

// getMembershipMonitor return a membership monitor
func (factory *factory) getMembershipMonitor() (membership.Monitor, error) {
return factory.getMembership()
}

func (factory *factory) getMembership() (membership.Monitor, error) {
// getMonitor return a membership monitor
func (factory *factory) getMonitor() (*monitor, error) {
var err error
factory.monOnce.Do(func() {
ctx, cancel := context.WithTimeout(context.Background(), persistenceOperationTimeout)
Expand All @@ -125,7 +121,7 @@ func (factory *factory) getMembership() (membership.Monitor, error) {
} else {
mrp := newService(rp, factory.config.MaxJoinDuration, factory.logger)

factory.membershipMonitor = newMonitor(
factory.monitor = newMonitor(
factory.serviceName,
factory.servicePortMap,
mrp,
Expand All @@ -136,7 +132,7 @@ func (factory *factory) getMembership() (membership.Monitor, error) {
}
})

return factory.membershipMonitor, err
return factory.monitor, err
}

func (factory *factory) broadcastAddressResolver() (string, error) {
Expand Down
2 changes: 1 addition & 1 deletion common/membership/ringpop/fx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func membershipMonitorProvider(
return nil, err
}

monitor, err := factory.getMembershipMonitor()
monitor, err := factory.getMonitor()
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion common/membership/ringpop/monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func newMonitor(
logger log.Logger,
metadataManager persistence.ClusterMetadataManager,
broadcastHostPortResolver func() (string, error),
) membership.Monitor {
) *monitor {
lifecycleCtx, lifecycleCancel := context.WithCancel(context.Background())
lifecycleCtx = headers.SetCallerInfo(
lifecycleCtx,
Expand Down
4 changes: 2 additions & 2 deletions common/membership/ringpop/test_cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ type testCluster struct {
hostUUIDs []string
hostAddrs []string
hostInfoList []membership.HostInfo
rings []membership.Monitor
rings []*monitor
channels []*tchannel.Channel
seedNode string
}
Expand Down Expand Up @@ -75,7 +75,7 @@ func newTestCluster(
hostUUIDs: make([]string, size),
hostAddrs: make([]string, size),
hostInfoList: make([]membership.HostInfo, size),
rings: make([]membership.Monitor, size),
rings: make([]*monitor, size),
channels: make([]*tchannel.Channel, size),
seedNode: seed,
}
Expand Down
2 changes: 0 additions & 2 deletions service/worker/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,6 @@ func (s *Service) Start() {
s.metricsHandler.Counter(metrics.RestartCount).Record(1)

s.clusterMetadata.Start()
s.membershipMonitor.Start()
s.namespaceRegistry.Start()

hostInfo, err := s.membershipMonitor.WhoAmI()
Expand Down Expand Up @@ -431,7 +430,6 @@ func (s *Service) Stop() {
s.perNamespaceWorkerManager.Stop()
s.workerManager.Stop()
s.namespaceRegistry.Stop()
s.membershipMonitor.Stop()
s.clusterMetadata.Stop()
s.persistenceBean.Close()
s.visibilityManager.Close()
Expand Down