Skip to content

Commit

Permalink
Response to comment networkservicemesh#1083 (comment)
Browse files Browse the repository at this point in the history
and also networkservicemesh#1083 (comment)

Signed-off-by: Ed Warnicke <hagbard@gmail.com>
  • Loading branch information
edwarnicke committed Sep 21, 2021
1 parent e452f53 commit 06adad8
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 29 deletions.
5 changes: 2 additions & 3 deletions pkg/networkservice/common/clientconn/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"google.golang.org/protobuf/types/known/emptypb"

"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
)

// NewClient - returns a clientconn chain element
Expand All @@ -41,15 +40,15 @@ type clientConnClient struct {

func (c *clientConnClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
if c.cc != nil {
_, _ = LoadOrStore(ctx, metadata.IsClient(c), c.cc)
_, _ = LoadOrStore(ctx, c.cc)
}
return next.Client(ctx).Request(ctx, request)
}

func (c *clientConnClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*emptypb.Empty, error) {
_, err := next.Client(ctx).Close(ctx, conn)
if c.cc != nil {
Delete(ctx, metadata.IsClient(c))
Delete(ctx)
}
return &emptypb.Empty{}, err
}
20 changes: 10 additions & 10 deletions pkg/networkservice/common/clientconn/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@ import (
type key struct{}

// Store sets the grpc.ClientConnInterface stored in per Connection.Id metadata.
func Store(ctx context.Context, isClient bool, cc grpc.ClientConnInterface) {
metadata.Map(ctx, isClient).Store(key{}, cc)
func Store(ctx context.Context, cc grpc.ClientConnInterface) {
metadata.Map(ctx, true).Store(key{}, cc)
}

// Delete deletes the grpc.ClientConnInterface stored in per Connection.Id metadata
func Delete(ctx context.Context, isClient bool) {
metadata.Map(ctx, isClient).Delete(key{})
func Delete(ctx context.Context) {
metadata.Map(ctx, true).Delete(key{})
}

// Load returns the grpc.ClientConnInterface stored in per Connection.Id metadata, or nil if no
// value is present.
// The ok result indicates whether value was found in the per Connection.Id metadata.
func Load(ctx context.Context, isClient bool) (value grpc.ClientConnInterface, ok bool) {
m := metadata.Map(ctx, isClient)
func Load(ctx context.Context) (value grpc.ClientConnInterface, ok bool) {
m := metadata.Map(ctx, true)
rawValue, ok := m.Load(key{})
if !ok {
return
Expand All @@ -53,8 +53,8 @@ func Load(ctx context.Context, isClient bool) (value grpc.ClientConnInterface, o
// LoadOrStore returns the existing grpc.ClientConnInterface stored in per Connection.Id metadata if present.
// Otherwise, it stores and returns the given nterface_types.InterfaceIndex.
// The loaded result is true if the value was loaded, false if stored.
func LoadOrStore(ctx context.Context, isClient bool, cc grpc.ClientConnInterface) (value grpc.ClientConnInterface, ok bool) {
rawValue, ok := metadata.Map(ctx, isClient).LoadOrStore(key{}, cc)
func LoadOrStore(ctx context.Context, cc grpc.ClientConnInterface) (value grpc.ClientConnInterface, ok bool) {
rawValue, ok := metadata.Map(ctx, true).LoadOrStore(key{}, cc)
if !ok {
return
}
Expand All @@ -64,8 +64,8 @@ func LoadOrStore(ctx context.Context, isClient bool, cc grpc.ClientConnInterface

// LoadAndDelete deletes the grpc.ClientConnInterface stored in per Connection.Id metadata,
// returning the previous value if any. The loaded result reports whether the key was present.
func LoadAndDelete(ctx context.Context, isClient bool) (value grpc.ClientConnInterface, ok bool) {
m := metadata.Map(ctx, isClient)
func LoadAndDelete(ctx context.Context) (value grpc.ClientConnInterface, ok bool) {
m := metadata.Map(ctx, true)
rawValue, ok := m.LoadAndDelete(key{})
if !ok {
return
Expand Down
5 changes: 2 additions & 3 deletions pkg/networkservice/common/connect/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"google.golang.org/protobuf/types/known/emptypb"

"github.com/networkservicemesh/sdk/pkg/networkservice/common/clientconn"
"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
)

type connectClient struct{}
Expand All @@ -36,7 +35,7 @@ func NewClient() networkservice.NetworkServiceClient {
}

func (c *connectClient) Request(ctx context.Context, request *networkservice.NetworkServiceRequest, opts ...grpc.CallOption) (*networkservice.Connection, error) {
cc, loaded := clientconn.Load(ctx, metadata.IsClient(c))
cc, loaded := clientconn.Load(ctx)
if !loaded {
return nil, errors.New("no grpc.ClientConnInterface provided")
}
Expand All @@ -45,7 +44,7 @@ func (c *connectClient) Request(ctx context.Context, request *networkservice.Net
}

func (c *connectClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*emptypb.Empty, error) {
cc, loaded := clientconn.Load(ctx, metadata.IsClient(c))
cc, loaded := clientconn.Load(ctx)
if !loaded {
return nil, errors.New("no grpc.ClientConnInterface provided")
}
Expand Down
18 changes: 9 additions & 9 deletions pkg/networkservice/common/dial/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ func (d *dialClient) Request(ctx context.Context, request *networkservice.Networ
return next.Client(ctx).Request(ctx, request, opts...)
}

cc, ccLoaded := clientconn.Load(ctx, metadata.IsClient(d))
cc, ccLoaded := clientconn.Load(ctx)

// If the previousClientURL doesn't match the current clientURL, close the old connection,
// which will trigger redial
previousClientURL, urlLoaded := loadClientURL(ctx, metadata.IsClient(d))
previousClientURL, urlLoaded := loadClientURL(ctx)
if ccLoaded && urlLoaded && previousClientURL.String() != clientURL.String() {
closer, ok := cc.(io.Closer)
if ok {
_ = closer.Close()
}
ccLoaded = false
clientconn.Delete(ctx, metadata.IsClient(d))
deleteClientURL(ctx, metadata.IsClient(d))
clientconn.Delete(ctx)
deleteClientURL(ctx)
}

// If we have no current cc, dial
Expand All @@ -96,7 +96,7 @@ func (d *dialClient) Request(ctx context.Context, request *networkservice.Networ
<-d.chainCtx.Done()
_ = cc.Close()
}()
clientconn.Store(ctx, metadata.IsClient(d), cc)
clientconn.Store(ctx, cc)
storeClientURL(ctx, metadata.IsClient(d), clientURL)
}
return next.Client(ctx).Request(ctx, request, opts...)
Expand All @@ -106,18 +106,18 @@ func (d *dialClient) Close(ctx context.Context, conn *networkservice.Connection,
_, err := next.Client(ctx).Close(ctx, conn, opts...)

clientURL := clienturlctx.ClientURL(ctx)
_, urlLoaded := loadClientURL(ctx, metadata.IsClient(d))
_, urlLoaded := loadClientURL(ctx)

// If we either have no URL for the close, or didn't previously have a URL
if clientURL == nil || !urlLoaded {
return &emptypb.Empty{}, err
}
cc, loaded := clientconn.Load(ctx, metadata.IsClient(d))
cc, loaded := clientconn.Load(ctx)
closer, ok := cc.(io.Closer)
if loaded && ok {
_ = closer.Close()
}
clientconn.Delete(ctx, metadata.IsClient(d))
deleteClientURL(ctx, metadata.IsClient(d))
clientconn.Delete(ctx)
deleteClientURL(ctx)
return &emptypb.Empty{}, err
}
8 changes: 4 additions & 4 deletions pkg/networkservice/common/dial/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ func storeClientURL(ctx context.Context, isClient bool, u *url.URL) {
}

// deleteClientURL deletes the *url.URL stored in per Connection.Id metadata
func deleteClientURL(ctx context.Context, isClient bool) {
metadata.Map(ctx, isClient).Delete(key{})
func deleteClientURL(ctx context.Context) {
metadata.Map(ctx, true).Delete(key{})
}

// loadClientURL returns the *url.URL stored in per Connection.Id metadata, or nil if no
// value is present.
// The ok result indicates whether value was found in the per Connection.Id metadata.
func loadClientURL(ctx context.Context, isClient bool) (value *url.URL, ok bool) {
rawValue, ok := metadata.Map(ctx, isClient).Load(key{})
func loadClientURL(ctx context.Context) (value *url.URL, ok bool) {
rawValue, ok := metadata.Map(ctx, true).Load(key{})
if !ok {
return
}
Expand Down

0 comments on commit 06adad8

Please sign in to comment.