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.

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

Extract interface from membership.HostInfo #3996

Merged
merged 2 commits into from
Mar 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 19 additions & 30 deletions common/membership/hostinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,41 +24,30 @@

package membership

// HostInfo is a type that contains the info about a temporal host
type HostInfo struct {
addr string // ip:port
labels map[string]string
// HostInfo represents the host of a Temporal service.
type HostInfo interface {
// Identity returns the unique identifier of the host.
// This may be the same as the address.
Identity() string
// GetAddress returns the socket address of the host (i.e. <ip>:<port>).
// This must be a valid gRPC address.
GetAddress() string
}

// NewHostInfo creates a new HostInfo instance
func NewHostInfo(addr string, labels map[string]string) *HostInfo {
if labels == nil {
labels = make(map[string]string)
}
return &HostInfo{
addr: addr,
labels: labels,
}
// NewHostInfoFromAddress creates a new HostInfo instance from a socket address.
func NewHostInfoFromAddress(address string) HostInfo {
return hostAddress(address)
}

// GetAddress returns the ip:port address
func (hi *HostInfo) GetAddress() string {
return hi.addr
}

// Identity implements ringpop's Membership interface
func (hi *HostInfo) Identity() string {
// for now we just use the address as the identity
return hi.addr
}
// hostAddress is a HostInfo implementation that uses a string as the address and identity.
type hostAddress string

// Label implements ringpop's Membership interface
func (hi *HostInfo) Label(key string) (value string, has bool) {
value, has = hi.labels[key]
return
// GetAddress returns the value of the hostAddress.
func (a hostAddress) GetAddress() string {
return string(a)
}

// SetLabel sets the label.
func (hi *HostInfo) SetLabel(key string, value string) {
hi.labels[key] = value
// Identity returns the value of the hostAddress.
func (a hostAddress) Identity() string {
return string(a)
}
4 changes: 2 additions & 2 deletions common/membership/hostinfo_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var HostInfoProviderModule = fx.Options(

type (
cachingHostInfoProvider struct {
hostInfo *HostInfo
hostInfo HostInfo
membershipMonitor Monitor
}
)
Expand All @@ -57,7 +57,7 @@ func (hip *cachingHostInfoProvider) Start() error {
return nil
}

func (hip *cachingHostInfoProvider) HostInfo() *HostInfo {
func (hip *cachingHostInfoProvider) HostInfo() HostInfo {
return hip.hostInfo
}

Expand Down
16 changes: 8 additions & 8 deletions common/membership/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,22 @@ type (

// ChangedEvent describes a change in membership
ChangedEvent struct {
HostsAdded []*HostInfo
HostsUpdated []*HostInfo
HostsRemoved []*HostInfo
HostsAdded []HostInfo
HostsUpdated []HostInfo
HostsRemoved []HostInfo
}

// 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)
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
// ring. This primitive is useful to carry out graceful host shutdown during deployments.
EvictSelf() error
Lookup(service primitives.ServiceName, key string) (*HostInfo, error)
Lookup(service primitives.ServiceName, key string) (HostInfo, error)
GetResolver(service primitives.ServiceName) (ServiceResolver, error)
// AddListener adds a listener for this service.
// The listener will get notified on the given
Expand All @@ -93,7 +93,7 @@ type (
// ServiceResolver provides membership information for a specific temporal service.
// It can be used to resolve which member host is responsible for serving a given key.
ServiceResolver interface {
Lookup(key string) (*HostInfo, error)
Lookup(key string) (HostInfo, error)
// AddListener adds a listener which will get notified on the given
// channel, whenever membership changes.
// @name: The name for identifying the listener
Expand All @@ -104,13 +104,13 @@ type (
// MemberCount returns host count in hashring for any particular role
MemberCount() int
// Members returns all host addresses in hashring for any particular role
Members() []*HostInfo
Members() []HostInfo
// Requests to rebuild the hash ring
RequestRefresh()
}

HostInfoProvider interface {
Start() error
HostInfo() *HostInfo
HostInfo() HostInfo
}
)
20 changes: 10 additions & 10 deletions common/membership/interfaces_mock.go

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

63 changes: 63 additions & 0 deletions common/membership/ringpop_hostinfo.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// The MIT License
//
// Copyright (c) 2020 Temporal Technologies Inc. All rights reserved.
//
// Copyright (c) 2020 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package membership

type ringpopHostInfo struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember you move the file to a subfolder ringpop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's in a later PR that depends on this change: #3999

addr string // ip:port
labels map[string]string
}

// newRingpopHostInfo creates a new *ringpopHostInfo instance
func newRingpopHostInfo(addr string, labels map[string]string) *ringpopHostInfo {
if labels == nil {
labels = make(map[string]string)
}
return &ringpopHostInfo{
addr: addr,
labels: labels,
}
}

// GetAddress returns the ip:port address
func (hi *ringpopHostInfo) GetAddress() string {
return hi.addr
}

// Identity implements ringpop's Membership interface
func (hi *ringpopHostInfo) Identity() string {
// for now we just use the address as the identity
return hi.addr
}

// Label implements ringpop's Membership interface
func (hi *ringpopHostInfo) Label(key string) (value string, has bool) {
value, has = hi.labels[key]
return
}

// SetLabel sets the label.
func (hi *ringpopHostInfo) SetLabel(key string, value string) {
hi.labels[key] = value
}
6 changes: 3 additions & 3 deletions common/membership/rp_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func (rpo *ringpopMonitor) Stop() {
// This is different from service address as we register ringpop handlers on a separate port.
// For this reason we need to lookup the port for the service and replace ringpop port with service port before
// returning HostInfo back.
func (rpo *ringpopMonitor) WhoAmI() (*HostInfo, error) {
func (rpo *ringpopMonitor) WhoAmI() (HostInfo, error) {
address, err := rpo.rp.WhoAmI()
if err != nil {
return nil, err
Expand All @@ -360,7 +360,7 @@ func (rpo *ringpopMonitor) WhoAmI() (*HostInfo, error) {
if err != nil {
return nil, err
}
return NewHostInfo(serviceAddress, labels.AsMap()), nil
return newRingpopHostInfo(serviceAddress, labels.AsMap()), nil
}

func (rpo *ringpopMonitor) EvictSelf() error {
Expand All @@ -375,7 +375,7 @@ func (rpo *ringpopMonitor) GetResolver(service primitives.ServiceName) (ServiceR
return ring, nil
}

func (rpo *ringpopMonitor) Lookup(service primitives.ServiceName, key string) (*HostInfo, error) {
func (rpo *ringpopMonitor) Lookup(service primitives.ServiceName, key string) (HostInfo, error) {
ring, err := rpo.GetResolver(service)
if err != nil {
return nil, err
Expand Down
6 changes: 3 additions & 3 deletions common/membership/rp_monitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ func (s *RpoSuite) testCompareMembers(curr []string, new []string, expectedDiff
if event != nil {
var diff []string
for _, a := range event.HostsAdded {
diff = append(diff, "+"+a.addr)
diff = append(diff, "+"+a.GetAddress())
}
for _, a := range event.HostsUpdated {
diff = append(diff, "~"+a.addr)
diff = append(diff, "~"+a.GetAddress())
}
for _, a := range event.HostsRemoved {
diff = append(diff, "-"+a.addr)
diff = append(diff, "-"+a.GetAddress())
}
s.ElementsMatch(expectedDiff, diff)
}
Expand Down
18 changes: 8 additions & 10 deletions common/membership/rp_service_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,15 @@ func (r *ringpopServiceResolver) RequestRefresh() {
}

// Lookup finds the host in the ring responsible for serving the given key
func (r *ringpopServiceResolver) Lookup(
key string,
) (*HostInfo, error) {
func (r *ringpopServiceResolver) Lookup(key string) (HostInfo, error) {

addr, found := r.ring().Lookup(key)
if !found {
r.RequestRefresh()
return nil, ErrInsufficientHosts
}

return NewHostInfo(addr, r.getLabelsMap()), nil
return newRingpopHostInfo(addr, r.getLabelsMap()), nil
}

func (r *ringpopServiceResolver) AddListener(
Expand Down Expand Up @@ -204,10 +202,10 @@ func (r *ringpopServiceResolver) MemberCount() int {
return r.ring().ServerCount()
}

func (r *ringpopServiceResolver) Members() []*HostInfo {
var servers []*HostInfo
func (r *ringpopServiceResolver) Members() []HostInfo {
var servers []HostInfo
for _, s := range r.ring().Servers() {
servers = append(servers, NewHostInfo(s, r.getLabelsMap()))
servers = append(servers, newRingpopHostInfo(s, r.getLabelsMap()))
}

return servers
Expand Down Expand Up @@ -274,7 +272,7 @@ func (r *ringpopServiceResolver) refreshNoLock() (*ChangedEvent, error) {

ring := newHashRing()
for _, addr := range addrs {
host := NewHostInfo(addr, r.getLabelsMap())
host := newRingpopHostInfo(addr, r.getLabelsMap())
ring.AddMembers(host)
}

Expand Down Expand Up @@ -373,13 +371,13 @@ func (r *ringpopServiceResolver) compareMembers(addrs []string) (map[string]stru
for _, addr := range addrs {
newMembersMap[addr] = struct{}{}
if _, ok := r.membersMap[addr]; !ok {
event.HostsAdded = append(event.HostsAdded, NewHostInfo(addr, r.getLabelsMap()))
event.HostsAdded = append(event.HostsAdded, newRingpopHostInfo(addr, r.getLabelsMap()))
changed = true
}
}
for addr := range r.membersMap {
if _, ok := newMembersMap[addr]; !ok {
event.HostsRemoved = append(event.HostsRemoved, NewHostInfo(addr, r.getLabelsMap()))
event.HostsRemoved = append(event.HostsRemoved, newRingpopHostInfo(addr, r.getLabelsMap()))
changed = true
}
}
Expand Down
6 changes: 3 additions & 3 deletions common/membership/rp_test_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func NewTestRingpopCluster(
logger.Error("Failed to build broadcast hostport", tag.Error(err))
return nil
}
cluster.hostInfoList[i] = HostInfo{addr: cluster.hostAddrs[i]}
cluster.hostInfoList[i] = newRingpopHostInfo(cluster.hostAddrs[i], nil)
}

// if seed node is already supplied, use it; if not, set it
Expand Down Expand Up @@ -209,9 +209,9 @@ func (c *TestRingpopCluster) GetHostAddrs() []string {
// the given addr, if it exists
func (c *TestRingpopCluster) FindHostByAddr(addr string) (HostInfo, bool) {
for _, hi := range c.hostInfoList {
if hi.addr == addr {
if hi.GetAddress() == addr {
return hi, true
}
}
return HostInfo{}, false
return nil, false
}
Loading