Skip to content

Commit

Permalink
[FABG-914] Use parent context in Discovery Dial
Browse files Browse the repository at this point in the history
Use the provided context as a parent context to the GRPC Dial so that we don't wait for a dial timeout when the server is down.

Signed-off-by: Bob Stasyszyn <[email protected]>
  • Loading branch information
bstasyszyn committed Oct 21, 2019
1 parent 4dd1938 commit c365e23
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 5 deletions.
4 changes: 3 additions & 1 deletion pkg/fab/comm/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ func NewConnection(ctx fabcontext.Client, url string, opts ...options.Opt) (*GRP
return nil, err
}

reqCtx, cancel := context.NewRequest(ctx, context.WithTimeout(params.connectTimeout))
reqCtx, cancel := context.NewRequest(ctx,
context.WithTimeout(params.connectTimeout),
context.WithParent(params.parentContext))
defer cancel()

commManager, ok := context.RequestCommManager(reqCtx)
Expand Down
21 changes: 17 additions & 4 deletions pkg/fab/comm/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,32 @@ SPDX-License-Identifier: Apache-2.0
package comm

import (
"context"
"testing"
"time"

eventmocks "github.com/hyperledger/fabric-sdk-go/pkg/fab/events/mocks"
fabmocks "github.com/hyperledger/fabric-sdk-go/pkg/fab/mocks"
mspmocks "github.com/hyperledger/fabric-sdk-go/pkg/msp/test/mockmsp"
"github.com/stretchr/testify/require"
)

func TestConnection(t *testing.T) {
context := newMockContext()
ctx := newMockContext()

_, err := NewConnection(context, "")
_, err := NewConnection(ctx, "")
if err == nil {
t.Fatal("expected error creating new connection with empty URL")
}
conn, err := NewConnection(context, peerURL)

conn, err := NewConnection(ctx, peerURL)
if err != nil {
t.Fatalf("error creating new connection: %s", err)
}
if conn.Closed() {
t.Fatal("expected connection to be open")
}
if _, err := context.Serialize(); err != nil {
if _, err := ctx.Serialize(); err != nil {
t.Fatal("error getting identity")
}

Expand All @@ -44,6 +47,16 @@ func TestConnection(t *testing.T) {
conn.Close()
}

func TestConnection_WithParentContext(t *testing.T) {
ctx := newMockContext()
reqCtx, cancel := context.WithCancel(context.Background())
cancel()
conn, err := NewConnection(ctx, "localhost:8978", WithParentContext(reqCtx))
require.Error(t, err)
require.Contains(t, err.Error(), context.Canceled.Error())
require.Nil(t, conn)
}

// Use the mock deliver server for testing
var testServer *eventmocks.MockDeliverServer
var endorserAddr []string
Expand Down
20 changes: 20 additions & 0 deletions pkg/fab/comm/connectionopts.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ SPDX-License-Identifier: Apache-2.0
package comm

import (
"context"
"crypto/x509"
"time"

Expand All @@ -23,6 +24,7 @@ type params struct {
failFast bool
insecure bool
connectTimeout time.Duration
parentContext context.Context
}

func defaultParams() *params {
Expand Down Expand Up @@ -77,6 +79,15 @@ func WithConnectTimeout(value time.Duration) options.Opt {
}
}

// WithParentContext sets the parent context
func WithParentContext(value context.Context) options.Opt {
return func(p options.Params) {
if setter, ok := p.(parentContextSetter); ok {
setter.SetParentContext(value)
}
}
}

// WithInsecure indicates to fall back to an insecure connection if the
// connection URL does not specify a protocol
func WithInsecure() options.Opt {
Expand Down Expand Up @@ -121,6 +132,11 @@ func (p *params) SetInsecure(value bool) {
p.insecure = value
}

func (p *params) SetParentContext(value context.Context) {
logger.Debugf("Setting parent context")
p.parentContext = value
}

type hostOverrideSetter interface {
SetHostOverride(value string)
}
Expand All @@ -145,6 +161,10 @@ type connectTimeoutSetter interface {
SetConnectTimeout(value time.Duration)
}

type parentContextSetter interface {
SetParentContext(value context.Context)
}

// OptsFromPeerConfig returns a set of connection options from the given peer config
func OptsFromPeerConfig(peerCfg *fab.PeerConfig) []options.Opt {

Expand Down
1 change: 1 addition & 0 deletions pkg/fab/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ func (c *Client) Send(ctx context.Context, req *Request, targets ...fab.PeerConf
func (c *Client) send(reqCtx context.Context, req *discclient.Request, target fab.PeerConfig) (discclient.Response, error) {
opts := comm.OptsFromPeerConfig(&target)
opts = append(opts, comm.WithConnectTimeout(c.ctx.EndpointConfig().Timeout(fab.DiscoveryConnection)))
opts = append(opts, comm.WithParentContext(reqCtx))

conn, err := comm.NewConnection(c.ctx, target.URL, opts...)
if err != nil {
Expand Down

0 comments on commit c365e23

Please sign in to comment.