FABGW-6 Use concurrent go routines for endorsement #2453
Conversation
|
/ci-run |
|
AZP build triggered! |
core/peer/config.go
Outdated
| @@ -274,7 +274,12 @@ func (c *Config) load() error { | |||
| } | |||
|
|
|||
| c.GatewayOptions = gateway.DefaultOptions() | |||
| c.GatewayOptions.Enabled = viper.GetBool("peer.gateway.enabled") | |||
| if viper.IsSet("peer.gateway.enabled") { | |||
There was a problem hiding this comment.
Since this is new code, the logic to create the gateway configuration should live in the gateway code. If possible (and I think we're finally at a level of the code that supports it), the viper instance should be passed to the constructor to avoid dealing with global state.
internal/peer/node/start.go
Outdated
| @@ -825,7 +825,7 @@ func serve(args []string) error { | |||
| logger.Info("Starting peer with Gateway enabled") | |||
| gatewayprotos.RegisterGatewayServer( | |||
| peerServer.Server(), | |||
| gateway.CreateServer(&gateway.EndorserServerAdapter{Server: serverEndorser}, discoveryService, peerInstance.GossipService.SelfMembershipInfo().Endpoint), | |||
| gateway.CreateServer(&gateway.EndorserServerAdapter{Server: serverEndorser}, discoveryService, peerInstance.GossipService.SelfMembershipInfo().Endpoint, coreConfig.GatewayOptions), | |||
There was a problem hiding this comment.
It's time to break this across multiple lines.
internal/pkg/gateway/api.go
Outdated
| }(endorser) | ||
| } | ||
|
|
||
| timeout := time.After(gs.options.EndorsementTimeout) |
There was a problem hiding this comment.
Under load, this can result in significant memory pressure. This problem is highlighted in the doc: The underlying Timer is not recovered by the garbage collector until the timer fires
That said, why do this? Why not decorate the context with a timeout and use the same one across all of the calls? (I'm guessing you copied this code from somewhere...) Once you know the calls have a proper timeout and cancelation, you could choose to keep using a select in a for loop or move to something simpler (like a WaitGroup) to wait for the calls to complete.
There was a problem hiding this comment.
rewritten to use context.WithTimeout, as suggested
internal/pkg/gateway/api_test.go
Outdated
| @@ -74,6 +76,7 @@ type contextKey string | |||
| func TestGateway(t *testing.T) { | |||
| const testChannel = "test_channel" | |||
| const testChaincode = "test_chaincode" | |||
| const endorsementTimeout = 1 * time.Second | |||
internal/pkg/gateway/api_test.go
Outdated
| signedProposal: validSignedProposal, | ||
| setupRegistry: func(reg *registry) { | ||
| reg.localEndorser.(*mocks.EndorserClient).ProcessProposalStub = func(context.Context, *peer.SignedProposal, ...grpc.CallOption) (*peer.ProposalResponse, error) { | ||
| time.Sleep(endorsementTimeout + 10*time.Millisecond) |
There was a problem hiding this comment.
We should rarely see a sleep in tests because there's no way to preempt them.
There was a problem hiding this comment.
this test is now redundant - removed
Replace the current sequential logic for getting multiple endorsements with concurrent fan-out/fan-in using go routines Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
Replace the current sequential logic for getting multiple endorsements with concurrent fan-out/fan-in using go routines
Add configurable endorsement timeout option to the global config and pass through to gateway
Signed-off-by: andrew-coleman andrew_coleman@uk.ibm.com