From fd764673982b0348e2280ddff69377c72dba8c0a Mon Sep 17 00:00:00 2001 From: Inbar Badian Date: Sun, 22 Jul 2018 15:13:36 +0300 Subject: [PATCH] [FAB-11170] Refactor discovery client cache changed name and type field maxMemorySize in signer.go, changed code of signer.go to skip putting in memory if maxSize is 0, changed parameter in CLI from 10 to 0 in stub.go, updated tests Change-Id: Ib3c81dafe09a3e267973399d53846aaae9c07689 Signed-off-by: Inbar Badian --- discovery/client/client.go | 2 +- discovery/client/client_test.go | 2 +- discovery/client/signer.go | 21 ++++++++++++--------- discovery/client/signer_test.go | 22 +++++++++++++++++++--- discovery/cmd/stub.go | 2 +- discovery/test/integration_test.go | 2 +- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/discovery/client/client.go b/discovery/client/client.go index 4a50f94b196..aa18d312a8f 100644 --- a/discovery/client/client.go +++ b/discovery/client/client.go @@ -535,7 +535,7 @@ type endorsementDescriptor struct { } // NewClient creates a new Client instance -func NewClient(createConnection Dialer, s Signer, signerCacheSize int) *Client { +func NewClient(createConnection Dialer, s Signer, signerCacheSize uint) *Client { return &Client{ createConnection: createConnection, signRequest: NewMemoizeSigner(s, signerCacheSize).Sign, diff --git a/discovery/client/client_test.go b/discovery/client/client_test.go index 97b4bd01fc7..49014d01b41 100644 --- a/discovery/client/client_test.go +++ b/discovery/client/client_test.go @@ -42,7 +42,7 @@ import ( ) const ( - signerCacheSize = 1 + signerCacheSize uint = 1 ) var ( diff --git a/discovery/client/signer.go b/discovery/client/signer.go index fddcbd1e53a..34b111b6882 100644 --- a/discovery/client/signer.go +++ b/discovery/client/signer.go @@ -16,7 +16,7 @@ import ( // MemoizeSigner signs messages with the same signature // if the message was signed recently type MemoizeSigner struct { - maxMemorySize int + maxEntries uint sync.RWMutex memory map[string][]byte sign Signer @@ -24,11 +24,11 @@ type MemoizeSigner struct { // NewMemoizeSigner creates a new MemoizeSigner that signs // message with the given sign function -func NewMemoizeSigner(signFunc Signer, maxMemorySize int) *MemoizeSigner { +func NewMemoizeSigner(signFunc Signer, maxEntries uint) *MemoizeSigner { return &MemoizeSigner{ - maxMemorySize: maxMemorySize, - memory: make(map[string][]byte), - sign: signFunc, + maxEntries: maxEntries, + memory: make(map[string][]byte), + sign: signFunc, } } @@ -57,25 +57,28 @@ func (ms *MemoizeSigner) lookup(msg []byte) ([]byte, bool) { } func (ms *MemoizeSigner) memorize(msg, signature []byte) { + if ms.maxEntries == 0 { + return + } ms.RLock() - shouldShrink := len(ms.memory) >= ms.maxMemorySize + shouldShrink := len(ms.memory) >= (int)(ms.maxEntries) ms.RUnlock() if shouldShrink { ms.shrinkMemory() } - ms.Lock() defer ms.Unlock() ms.memory[msgDigest(msg)] = signature + } // evict evicts random messages from memory -// until its size is smaller than maxMemorySize +// until its size is smaller than maxEntries func (ms *MemoizeSigner) shrinkMemory() { ms.Lock() defer ms.Unlock() - for len(ms.memory) > ms.maxMemorySize { + for len(ms.memory) > (int)(ms.maxEntries) { ms.evictFromMemory() } } diff --git a/discovery/client/signer_test.go b/discovery/client/signer_test.go index b27a8a6e5ec..c882ca93475 100644 --- a/discovery/client/signer_test.go +++ b/discovery/client/signer_test.go @@ -7,6 +7,8 @@ SPDX-License-Identifier: Apache-2.0 package discovery import ( + "crypto/rand" + "io" "sync" "sync/atomic" "testing" @@ -37,7 +39,7 @@ func TestSameMessage(t *testing.T) { } func TestDifferentMessages(t *testing.T) { - n := 5 + var n uint = 5 var signedInvokedCount uint32 sign := func(msg []byte) ([]byte, error) { atomic.AddUint32(&signedInvokedCount, 1) @@ -45,9 +47,9 @@ func TestDifferentMessages(t *testing.T) { } ms := NewMemoizeSigner(sign, n) - parallelSignRange := func(start, end int) { + parallelSignRange := func(start, end uint) { var wg sync.WaitGroup - wg.Add(end - start) + wg.Add((int)(end - start)) for i := start; i < end; i++ { i := i go func() { @@ -86,3 +88,17 @@ func TestFailure(t *testing.T) { _, err := ms.Sign([]byte{1, 2, 3}) assert.Equal(t, "something went wrong", err.Error()) } + +func TestNotSavingInMem(t *testing.T) { + sign := func(_ []byte) ([]byte, error) { + b := make([]byte, 30) + _, err := io.ReadFull(rand.Reader, b) + assert.NoError(t, err) + return b, nil + } + ms := NewMemoizeSigner(sign, 0) + sig1, _ := ms.sign(([]byte)("aa")) + sig2, _ := ms.sign(([]byte)("aa")) + assert.NotEqual(t, sig1, sig2) + +} diff --git a/discovery/cmd/stub.go b/discovery/cmd/stub.go index 601de950391..7a815e67410 100644 --- a/discovery/cmd/stub.go +++ b/discovery/cmd/stub.go @@ -61,7 +61,7 @@ func (stub *ClientStub) Send(server string, conf common.Config, req *discovery.R timeout, cancel := context.WithTimeout(context.Background(), defaultTimeout) defer cancel() - disc := discovery.NewClient(comm.NewDialer(server), signer.Sign, 10) + disc := discovery.NewClient(comm.NewDialer(server), signer.Sign, 0) resp, err := disc.Send(timeout, req, &AuthInfo{ ClientIdentity: signer.Creator, diff --git a/discovery/test/integration_test.go b/discovery/test/integration_test.go index 16c77de7e95..6438014d770 100644 --- a/discovery/test/integration_test.go +++ b/discovery/test/integration_test.go @@ -494,7 +494,7 @@ func createClientAndService(t *testing.T, testdir string) (*client, *service) { assert.NoError(t, err) wrapperClient := &client{AuthInfo: authInfo, conn: conn} - signerCacheSize := 10 + var signerCacheSize uint = 10 c := disc.NewClient(wrapperClient.newConnection, signer.Sign, signerCacheSize) wrapperClient.Client = c service := &service{Server: gRPCServer.Server(), lc: lc, sup: sup}