[FAB-4473] mod cc cache strategy to store minimal data

The chaincode cache implementation stores the entire CCPackage
where only the InstantiationPolicy (IP) is needed. In this
CR we cache ChaincodeData extracted from CCPackage as that
contains IP and is much smaller than a typical package.

. removed TestCCInfoCachePeerInstance but left a bit of coverage
  into another UT
. fixed a bug introduced by rebase and add a couple of logs
  statements
Change-Id: Ic1fa64c7a50d3ea3247adb63be5b89f0b2af60cd
Signed-off-by: Srinivasan Muralidharan <[email protected]>

Author: Srinivasan Muralidharan

core/common/ccprovider/ccinfocache.go

@@ -19,88 +19,52 @@ package ccprovider
 import (
 	"fmt"
 	"sync"
-
-	"github.com/hyperledger/fabric/protos/peer"
 )
 
-// ccInfoCacheImpl implements CCInfoProvider by providing an in-memory
-// cache layer on top of the internal CCInfoProvider instance
+// ccInfoCacheImpl implements in-memory cache for ChaincodeData
+// needed by endorser to verify if the local instantiation policy
+// matches the instantiation policy on a channel before honoring
+// an invoke
 type ccInfoCacheImpl struct {
 	sync.RWMutex
 
-	cache map[string]CCPackage
-	ccfs  CCInfoProvider
+	cache        map[string]*ChaincodeData
+	cacheSupport CCCacheSupport
 }
 
 // NewCCInfoCache returns a new cache on top of the supplied CCInfoProvider instance
-func NewCCInfoCache(ccfs CCInfoProvider) CCInfoProvider {
+func NewCCInfoCache(cs CCCacheSupport) *ccInfoCacheImpl {
 	return &ccInfoCacheImpl{
-		cache: make(map[string]CCPackage),
-		ccfs:  ccfs,
+		cache:        make(map[string]*ChaincodeData),
+		cacheSupport: cs,
 	}
 }
 
-func (c *ccInfoCacheImpl) GetChaincode(ccname string, ccversion string) (CCPackage, error) {
+func (c *ccInfoCacheImpl) GetChaincodeData(ccname string, ccversion string) (*ChaincodeData, error) {
 	// c.cache is guaranteed to be non-nil
 
 	key := ccname + "/" + ccversion
 
 	c.RLock()
-	ccpack, in := c.cache[key]
+	ccdata, in := c.cache[key]
 	c.RUnlock()
 
 	if !in {
 		var err error
 
 		// the chaincode data is not in the cache
 		// try to look it up from the file system
-		ccpack, err = c.ccfs.GetChaincode(ccname, ccversion)
+		ccpack, err := c.cacheSupport.GetChaincode(ccname, ccversion)
 		if err != nil || ccpack == nil {
 			return nil, fmt.Errorf("cannot retrieve package for chaincode %s/%s, error %s", ccname, ccversion, err)
 		}
 
-		// we have a non-nil CCPackage, put it in the cache
+		// we have a non-nil ChaincodeData, put it in the cache
 		c.Lock()
-		c.cache[key] = ccpack
+		ccdata = ccpack.GetChaincodeData()
+		c.cache[key] = ccdata
 		c.Unlock()
 	}
 
-	return ccpack, nil
-}
-
-func (c *ccInfoCacheImpl) PutChaincode(depSpec *peer.ChaincodeDeploymentSpec) (CCPackage, error) {
-	// c.cache is guaranteed to be non-nil
-
-	ccname := depSpec.ChaincodeSpec.ChaincodeId.Name
-	ccversion := depSpec.ChaincodeSpec.ChaincodeId.Version
-
-	if ccname == "" {
-		return nil, fmt.Errorf("the chaincode name cannot be an empty string")
-	}
-
-	if ccversion == "" {
-		return nil, fmt.Errorf("the chaincode version cannot be an empty string")
-	}
-
-	key := ccname + "/" + ccversion
-
-	c.RLock()
-	_, in := c.cache[key]
-	c.RUnlock()
-
-	if in {
-		return nil, fmt.Errorf("attempted to put chaincode data twice for %s/%s", ccname, ccversion)
-	}
-
-	ccpack, err := c.ccfs.PutChaincode(depSpec)
-	if err != nil || ccpack == nil {
-		return nil, fmt.Errorf("PutChaincodeIntoFS failed, error %s", err)
-	}
-
-	// we have a non-nil CCPackage, put it in the cache
-	c.Lock()
-	c.cache[key] = ccpack
-	c.Unlock()
-
-	return ccpack, nil
+	return ccdata, nil
 }

core/common/ccprovider/ccinfocache_test.go

@@ -20,7 +20,6 @@ import (
 	"archive/tar"
 	"bytes"
 	"compress/gzip"
-	"errors"
 	"io/ioutil"
 	"path/filepath"
 	"testing"
@@ -70,27 +69,7 @@ func buildPackage(name string, path string, version string, initArgs [][]byte) (
 }
 
 type mockCCInfoFSStorageMgrImpl struct {
-	CCMap           map[string]CCPackage
-	putchaincodeErr bool
-}
-
-func (m *mockCCInfoFSStorageMgrImpl) PutChaincode(depSpec *peer.ChaincodeDeploymentSpec) (CCPackage, error) {
-	if m.putchaincodeErr {
-		return nil, errors.New("Error putting the chaincode")
-	}
-
-	buf, err := proto.Marshal(depSpec)
-	if err != nil {
-		return nil, err
-	}
-	cccdspack := &CDSPackage{}
-	if _, err := cccdspack.InitFromBuffer(buf); err != nil {
-		return nil, err
-	}
-
-	m.CCMap[depSpec.ChaincodeSpec.ChaincodeId.Name+depSpec.ChaincodeSpec.ChaincodeId.Version] = cccdspack
-
-	return cccdspack, nil
+	CCMap map[string]CCPackage
 }
 
 func (m *mockCCInfoFSStorageMgrImpl) GetChaincode(ccname string, ccversion string) (CCPackage, error) {
@@ -109,7 +88,7 @@ func TestCCInfoCache(t *testing.T) {
 	// test the get side
 
 	// the cc data is not yet in the cache
-	_, err := cccache.GetChaincode(ccname, ccver)
+	_, err := cccache.GetChaincodeData(ccname, ccver)
 	assert.Error(t, err)
 
 	// put it in the file system
@@ -118,11 +97,11 @@ func TestCCInfoCache(t *testing.T) {
 	ccinfoFs.CCMap[ccname+ccver] = pack
 
 	// expect it to be in the cache now
-	cd1, err := cccache.GetChaincode(ccname, ccver)
+	cd1, err := cccache.GetChaincodeData(ccname, ccver)
 	assert.NoError(t, err)
 
 	// it should still be in the cache
-	cd2, err := cccache.GetChaincode(ccname, ccver)
+	cd2, err := cccache.GetChaincodeData(ccname, ccver)
 	assert.NoError(t, err)
 
 	// they are not null
@@ -131,21 +110,21 @@ func TestCCInfoCache(t *testing.T) {
 
 	// test the put side now..
 	ccver = "2.0"
-
-	// create a dep spec to put
-	ds, err := getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
+	// put it in the file system
+	pack, err = buildPackage(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
 	assert.NoError(t, err)
+	ccinfoFs.CCMap[ccname+ccver] = pack
 
-	// put it
-	_, err = cccache.PutChaincode(ds)
+	// create a dep spec to put
+	_, err = getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
 	assert.NoError(t, err)
 
-	// expect it to be in the cache
-	cd1, err = cccache.GetChaincode(ccname, ccver)
+	// expect it to be cached
+	cd1, err = cccache.GetChaincodeData(ccname, ccver)
 	assert.NoError(t, err)
 
 	// it should still be in the cache
-	cd2, err = cccache.GetChaincode(ccname, ccver)
+	cd2, err = cccache.GetChaincodeData(ccname, ccver)
 	assert.NoError(t, err)
 
 	// they are not null
@@ -159,36 +138,27 @@ func TestPutChaincode(t *testing.T) {
 	ccpath := "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example02"
 
 	ccinfoFs := &mockCCInfoFSStorageMgrImpl{CCMap: map[string]CCPackage{}}
-	cccache := NewCCInfoCache(ccinfoFs)
+	NewCCInfoCache(ccinfoFs)
 
 	// Error case 1: ccname is empty
 	// create a dep spec to put
-	ds, err := getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
+	_, err := getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
 	assert.NoError(t, err)
-	_, err = cccache.PutChaincode(ds)
-	assert.Error(t, err)
-	assert.Equal(t, "the chaincode name cannot be an empty string", err.Error(), "Unexpected error received")
 
 	// Error case 2: ccver is empty
 	ccname = "foo"
 	ccver = ""
-	ds, err = getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
+	_, err = getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
 	assert.NoError(t, err)
-	_, err = cccache.PutChaincode(ds)
-	assert.Error(t, err)
-	assert.Equal(t, "the chaincode version cannot be an empty string", err.Error(), "Unexpected error received")
 
 	// Error case 3: ccfs.PutChainCode returns an error
-	ccinfoFs = &mockCCInfoFSStorageMgrImpl{CCMap: map[string]CCPackage{}, putchaincodeErr: true}
-	cccache = NewCCInfoCache(ccinfoFs)
+	ccinfoFs = &mockCCInfoFSStorageMgrImpl{CCMap: map[string]CCPackage{}}
+	NewCCInfoCache(ccinfoFs)
 
 	ccname = "foo"
 	ccver = "1.0"
-	ds, err = getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
+	_, err = getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
 	assert.NoError(t, err)
-	_, err = cccache.PutChaincode(ds)
-	assert.Error(t, err)
-	assert.Contains(t, err.Error(), "PutChaincodeIntoFS failed", "Unexpected error received")
 }
 
 // here we test the peer's built-in cache after enabling it
@@ -208,44 +178,16 @@ func TestCCInfoFSPeerInstance(t *testing.T) {
 	// put it
 	err = PutChaincodeIntoFS(ds)
 	assert.NoError(t, err)
-}
-
-// here we test the peer's built-in cache after enabling it
-func TestCCInfoCachePeerInstance(t *testing.T) {
-	// enable the cache first: it's disabled by default
-	EnableCCInfoCache()
-
-	ccname := "foo"
-	ccver := "1.0"
-	ccpath := "github.com/hyperledger/fabric/examples/chaincode/go/chaincode_example02"
-
-	// the cc data is not yet in the cache
-	_, err := GetChaincodeFromFS(ccname, ccver)
-	assert.Error(t, err)
-
-	// create a dep spec to put
-	ds, err := getDepSpec(ccname, ccpath, ccver, [][]byte{[]byte("init"), []byte("a"), []byte("100"), []byte("b"), []byte("200")})
-	assert.NoError(t, err)
-
-	// put it
-	err = PutChaincodeIntoFS(ds)
-	assert.NoError(t, err)
-
-	// Check if chain code package exists
-	exists, err := ChaincodePackageExists(ccname, ccver)
-	assert.NoError(t, err)
-	assert.True(t, exists, "ChaincodePackageExists should have returned true for an existing package")
 
 	// Get all installed chaincodes, it should not return 0 chain codes
 	resp, err := GetInstalledChaincodes()
 	assert.NoError(t, err)
 	assert.NotNil(t, resp)
 	assert.NotZero(t, len(resp.Chaincodes), "GetInstalledChaincodes should not have returned 0 chain codes")
 
-	// expect it to be in the cache
-	cd, err := GetChaincodeFromFS(ccname, ccver)
+	//get chaincode data
+	_, err = GetChaincodeData(ccname, ccver)
 	assert.NoError(t, err)
-	assert.NotNil(t, cd)
 }
 
 func TestGetInstalledChaincodesErrorPaths(t *testing.T) {

core/common/ccprovider/ccprovider.go

@@ -112,20 +112,13 @@ func ChaincodePackageExists(ccname string, ccversion string) (bool, error) {
 	return false, err
 }
 
-// CCInfoProvider is responsible to provide backend storage for information
-// about chaincodes. Multiple implementations can persist data to a file system
-// or store them in a cache
-type CCInfoProvider interface {
-	// GetChaincode returns information for the chaincode with the
-	// supplied name and version
+type CCCacheSupport interface {
+	//GetChaincode is needed by the cache to get chaincode data
 	GetChaincode(ccname string, ccversion string) (CCPackage, error)
-
-	// PutChaincode stores the supplied chaincode info
-	PutChaincode(depSpec *pb.ChaincodeDeploymentSpec) (CCPackage, error)
 }
 
-// CCInfoFSStorageMgr is an implementation of CCInfoProvider
-// backed by the file system
+// CCInfoFSImpl provides the implementation for CC on the FS and the access to it
+// It implements CCCacheSupport
 type CCInfoFSImpl struct{}
 
 // GetChaincodeFromFS this is a wrapper for hiding package implementation.
@@ -195,40 +188,39 @@ func EnableCCInfoCache() {
 	ccInfoCacheEnabled = true
 }
 
-// GetChaincodeFromFS retrieves chaincode information from the cache (or the
-// file system in case of a cache miss) if the cache is enabled, or directly
-// from the file system otherwise
+// GetChaincodeFromFS retrieves chaincode information from the file system
 func GetChaincodeFromFS(ccname string, ccversion string) (CCPackage, error) {
-	if ccInfoCacheEnabled {
-		return ccInfoCache.GetChaincode(ccname, ccversion)
-	} else {
-		return ccInfoFSProvider.GetChaincode(ccname, ccversion)
-	}
+	return ccInfoFSProvider.GetChaincode(ccname, ccversion)
 }
 
 // PutChaincodeIntoFS puts chaincode information in the file system (and
 // also in the cache to prime it) if the cache is enabled, or directly
 // from the file system otherwise
 func PutChaincodeIntoFS(depSpec *pb.ChaincodeDeploymentSpec) error {
+	_, err := ccInfoFSProvider.PutChaincode(depSpec)
+	return err
+}
+
+// GetChaincodeData gets chaincode data from cache if there's one
+func GetChaincodeData(ccname string, ccversion string) (*ChaincodeData, error) {
 	if ccInfoCacheEnabled {
-		_, err := ccInfoCache.PutChaincode(depSpec)
-		return err
+		ccproviderLogger.Debugf("Getting chaincode data for <%s, %s> from cache", ccname, ccversion)
+		return ccInfoCache.GetChaincodeData(ccname, ccversion)
+	}
+	if ccpack, err := ccInfoFSProvider.GetChaincode(ccname, ccversion); err != nil {
+		return nil, err
 	} else {
-		_, err := ccInfoFSProvider.PutChaincode(depSpec)
-		return err
+		ccproviderLogger.Infof("Putting chaincode data for <%s, %s> into cache", ccname, ccversion)
+		return ccpack.GetChaincodeData(), nil
 	}
 }
 
 func CheckInsantiationPolicy(name, version string, cdLedger *ChaincodeData) error {
-	// we retrieve info about this chaincode from the file system
-	ccpack, err := GetChaincodeFromFS(name, version)
+	ccdata, err := GetChaincodeData(name, version)
 	if err != nil {
-		return fmt.Errorf("Chaincode data for cc %s/%s was not found, error %s", name, version, err)
+		return err
 	}
 
-	// ccpack is guaranteed to be non-nil
-	cdLocalFS := ccpack.GetChaincodeData()
-
 	// we have the info from the fs, check that the policy
 	// matches the one on the file system if one was specified;
 	// this check is required because the admin of this peer
@@ -241,8 +233,8 @@ func CheckInsantiationPolicy(name, version string, cdLedger *ChaincodeData) erro
 	// happen, i.e. that the peer will refuse to invoke the
 	// chaincode under these conditions. More info on
 	// https://jira.hyperledger.org/browse/FAB-3156
-	if cdLocalFS.InstantiationPolicy != nil {
-		if !bytes.Equal(cdLocalFS.InstantiationPolicy, cdLedger.InstantiationPolicy) {
+	if ccdata.InstantiationPolicy != nil {
+		if !bytes.Equal(ccdata.InstantiationPolicy, cdLedger.InstantiationPolicy) {
 			return fmt.Errorf("Instantiation policy mismatch for cc %s/%s", name, version)
 		}
 	}