[FAB-15488] Refactor viper in gossip/privdata

Refactor viper calls in gossip/privdata package, creating
privdataConfig structure to hold all viper configurations.

Change-Id: I5749213b961378e6435bf2e8092f91c15de009db
Signed-off-by: Chongxin Luo <Chongxin.Luo@ibm.com>

Author: DereckLuo

gossip/privdata/config.go

@@ -0,0 +1,53 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package privdata
+
+import (
+	"time"
+
+	"github.com/spf13/viper"
+)
+
+const (
+	reconcileSleepIntervalDefault = time.Minute * 1
+	reconcileBatchSizeDefault     = 10
+)
+
+// PrivdataConfig is the struct that defines the Gossip Privdata configurations.
+type PrivdataConfig struct {
+	// ReconcileSleepInterval determines the time reconciler sleeps from end of an interation until the beginning of the next
+	// reconciliation iteration.
+	ReconcileSleepInterval time.Duration
+	// ReconcileBatchSize determines the maximum batch size of missing private data that will be reconciled in a single iteration.
+	ReconcileBatchSize int
+	// ReconciliationEnabled is a flag that indicates whether private data reconciliation is enabled or not.
+	ReconciliationEnabled bool
+}
+
+// GlobalConfig obtains a set of configuration from viper, build and returns the config struct.
+func GlobalConfig() *PrivdataConfig {
+	c := &PrivdataConfig{}
+	c.loadPrivDataConfig()
+	return c
+}
+
+func (c *PrivdataConfig) loadPrivDataConfig() {
+	c.ReconcileSleepInterval = viper.GetDuration("peer.gossip.pvtData.reconcileSleepInterval")
+	if c.ReconcileSleepInterval == 0 {
+		logger.Warning("Configuration key peer.gossip.pvtData.reconcileSleepInterval isn't set, defaulting to", reconcileSleepIntervalDefault)
+		c.ReconcileSleepInterval = reconcileSleepIntervalDefault
+	}
+
+	c.ReconcileBatchSize = viper.GetInt("peer.gossip.pvtData.reconcileBatchSize")
+	if c.ReconcileBatchSize == 0 {
+		logger.Warning("Configuration key peer.gossip.pvtData.reconcileBatchSize isn't set, defaulting to", reconcileBatchSizeDefault)
+		c.ReconcileBatchSize = reconcileBatchSizeDefault
+	}
+
+	c.ReconciliationEnabled = viper.GetBool("peer.gossip.pvtData.reconciliationEnabled")
+
+}

gossip/privdata/config_test.go

@@ -0,0 +1,48 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package privdata_test
+
+import (
+	"testing"
+	"time"
+
+	"github.com/hyperledger/fabric/gossip/privdata"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGlobalConfig(t *testing.T) {
+	viper.Reset()
+	// Capture the configuration from viper
+	viper.Set("peer.gossip.pvtData.reconcileSleepInterval", "10s")
+	viper.Set("peer.gossip.pvtData.reconcileBatchSize", 10)
+	viper.Set("peer.gossip.pvtData.reconciliationEnabled", true)
+
+	coreConfig := privdata.GlobalConfig()
+
+	expectedConfig := &privdata.PrivdataConfig{
+		ReconcileSleepInterval: 10 * time.Second,
+		ReconcileBatchSize:     10,
+		ReconciliationEnabled:  true,
+	}
+
+	assert.Equal(t, coreConfig, expectedConfig)
+}
+
+func TestGlobalConfigDefaults(t *testing.T) {
+	viper.Reset()
+
+	coreConfig := privdata.GlobalConfig()
+
+	expectedConfig := &privdata.PrivdataConfig{
+		ReconcileSleepInterval: time.Minute,
+		ReconcileBatchSize:     10,
+		ReconciliationEnabled:  false,
+	}
+
+	assert.Equal(t, coreConfig, expectedConfig)
+}

gossip/privdata/coordinator_test.go

@@ -46,7 +46,7 @@ func init() {
 
 var testConfig = CoordinatorConfig{
 	PullRetryThreshold:      time.Second * 3,
-	TransientBlockRetention: TransientBlockRetentionDefault,
+	TransientBlockRetention: 1000,
 }
 
 // CollectionCriteria aggregates criteria of

gossip/privdata/reconcile.go

@@ -55,14 +55,15 @@ type PvtDataReconciler interface {
 }
 
 type Reconciler struct {
-	channel string
-	metrics *metrics.PrivdataMetrics
-	config  *ReconcilerConfig
+	channel                string
+	metrics                *metrics.PrivdataMetrics
+	ReconcileSleepInterval time.Duration
+	ReconcileBatchSize     int
+	stopChan               chan struct{}
+	startOnce              sync.Once
+	stopOnce               sync.Once
 	ReconciliationFetcher
 	committer.Committer
-	stopChan  chan struct{}
-	startOnce sync.Once
-	stopOnce  sync.Once
 }
 
 // NoOpReconciler non functional reconciler to be used
@@ -79,24 +80,18 @@ func (*NoOpReconciler) Stop() {
 	// do nothing
 }
 
-// ReconcilerConfig holds config flags that are read from core.yaml
-type ReconcilerConfig struct {
-	SleepInterval time.Duration
-	BatchSize     int
-	IsEnabled     bool
-}
-
 // NewReconciler creates a new instance of reconciler
 func NewReconciler(channel string, metrics *metrics.PrivdataMetrics, c committer.Committer,
-	fetcher ReconciliationFetcher, config *ReconcilerConfig) *Reconciler {
+	fetcher ReconciliationFetcher, config *PrivdataConfig) *Reconciler {
 	logger.Debug("Private data reconciliation is enabled")
 	return &Reconciler{
-		channel:               channel,
-		metrics:               metrics,
-		config:                config,
-		Committer:             c,
-		ReconciliationFetcher: fetcher,
-		stopChan:              make(chan struct{}),
+		channel:                channel,
+		metrics:                metrics,
+		ReconcileSleepInterval: config.ReconcileSleepInterval,
+		ReconcileBatchSize:     config.ReconcileBatchSize,
+		Committer:              c,
+		ReconciliationFetcher:  fetcher,
+		stopChan:               make(chan struct{}),
 	}
 }
 
@@ -117,7 +112,7 @@ func (r *Reconciler) run() {
 		select {
 		case <-r.stopChan:
 			return
-		case <-time.After(r.config.SleepInterval):
+		case <-time.After(r.ReconcileSleepInterval):
 			logger.Debug("Start reconcile missing private info")
 			if err := r.reconcile(); err != nil {
 				logger.Error("Failed to reconcile missing private info, error: ", err.Error())
@@ -143,7 +138,7 @@ func (r *Reconciler) reconcile() error {
 	defer r.reportReconciliationDuration(time.Now())
 
 	for {
-		missingPvtDataInfo, err := missingPvtDataTracker.GetMissingPvtDataInfoForMostRecentBlocks(r.config.BatchSize)
+		missingPvtDataInfo, err := missingPvtDataTracker.GetMissingPvtDataInfoForMostRecentBlocks(r.ReconcileBatchSize)
 		if err != nil {
 			logger.Error("reconciliation error when trying to get missing pvt data info recent blocks:", err)
 			return err

gossip/privdata/reconcile_test.go

@@ -40,9 +40,13 @@ func TestNoItemsToReconcile(t *testing.T) {
 	committer.On("GetMissingPvtDataTracker").Return(missingPvtDataTracker, nil)
 	fetcher.On("FetchReconciledItems", mock.Anything).Return(nil, errors.New("this function shouldn't be called"))
 
-	r := &Reconciler{channel: "", metrics: metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
-		config:                &ReconcilerConfig{SleepInterval: time.Minute, BatchSize: 1, IsEnabled: true},
-		ReconciliationFetcher: fetcher, Committer: committer}
+	r := &Reconciler{
+		channel:                "",
+		metrics:                metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
+		ReconcileSleepInterval: time.Minute,
+		ReconcileBatchSize:     1,
+		ReconciliationFetcher:  fetcher, Committer: committer,
+	}
 	err := r.reconcile()
 
 	assert.NoError(t, err)
@@ -78,9 +82,13 @@ func TestNotReconcilingWhenCollectionConfigNotAvailable(t *testing.T) {
 		fetchCalled = true
 	}).Return(nil, errors.New("called with no digests"))
 
-	r := &Reconciler{channel: "", metrics: metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
-		config:                &ReconcilerConfig{SleepInterval: time.Minute, BatchSize: 1, IsEnabled: true},
-		ReconciliationFetcher: fetcher, Committer: committer}
+	r := &Reconciler{
+		channel:                "",
+		metrics:                metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
+		ReconcileSleepInterval: time.Minute,
+		ReconcileBatchSize:     1,
+		ReconciliationFetcher:  fetcher, Committer: committer,
+	}
 	err := r.reconcile()
 
 	assert.Error(t, err)
@@ -160,9 +168,13 @@ func TestReconciliationHappyPathWithoutScheduler(t *testing.T) {
 	testMetricProvider := gmetricsmocks.TestUtilConstructMetricProvider()
 	metrics := metrics.NewGossipMetrics(testMetricProvider.FakeProvider).PrivdataMetrics
 
-	r := &Reconciler{channel: "mychannel", metrics: metrics,
-		config:                &ReconcilerConfig{SleepInterval: time.Minute, BatchSize: 1, IsEnabled: true},
-		ReconciliationFetcher: fetcher, Committer: committer}
+	r := &Reconciler{
+		channel:                "mychannel",
+		metrics:                metrics,
+		ReconcileSleepInterval: time.Minute,
+		ReconcileBatchSize:     1,
+		ReconciliationFetcher:  fetcher, Committer: committer,
+	}
 	err := r.reconcile()
 
 	assert.NoError(t, err)
@@ -247,8 +259,16 @@ func TestReconciliationHappyPathWithScheduler(t *testing.T) {
 		wg.Done()
 	}).Return([]*ledger.PvtdataHashMismatch{}, nil)
 
-	r := NewReconciler("", metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics, committer, fetcher,
-		&ReconcilerConfig{SleepInterval: time.Millisecond * 100, BatchSize: 1, IsEnabled: true})
+	r := NewReconciler(
+		"",
+		metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
+		committer,
+		fetcher,
+		&PrivdataConfig{
+			ReconcileSleepInterval: time.Millisecond * 100,
+			ReconcileBatchSize:     1,
+			ReconciliationEnabled:  true,
+		})
 	r.Start()
 	wg.Wait()
 	r.Stop()
@@ -357,8 +377,16 @@ func TestReconciliationPullingMissingPrivateDataAtOnePass(t *testing.T) {
 		wg.Done()
 	}).Return([]*ledger.PvtdataHashMismatch{}, nil)
 
-	r := NewReconciler("", metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics, committer, fetcher,
-		&ReconcilerConfig{SleepInterval: time.Millisecond * 100, BatchSize: 1, IsEnabled: true})
+	r := NewReconciler(
+		"",
+		metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
+		committer,
+		fetcher,
+		&PrivdataConfig{
+			ReconcileSleepInterval: time.Millisecond * 100,
+			ReconcileBatchSize:     1,
+			ReconciliationEnabled:  true,
+		})
 	r.Start()
 	<-stopC
 	r.Stop()
@@ -437,9 +465,13 @@ func TestReconciliationFailedToCommit(t *testing.T) {
 
 	committer.On("CommitPvtDataOfOldBlocks", mock.Anything).Return(nil, errors.New("failed to commit"))
 
-	r := &Reconciler{channel: "", metrics: metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
-		config:                &ReconcilerConfig{SleepInterval: time.Minute, BatchSize: 1, IsEnabled: true},
-		ReconciliationFetcher: fetcher, Committer: committer}
+	r := &Reconciler{
+		channel:                "",
+		metrics:                metrics.NewGossipMetrics(&disabled.Provider{}).PrivdataMetrics,
+		ReconcileSleepInterval: time.Minute,
+		ReconcileBatchSize:     1,
+		ReconciliationFetcher:  fetcher, Committer: committer,
+	}
 	err := r.reconcile()
 
 	assert.Error(t, err)
@@ -452,16 +484,24 @@ func TestFailuresWhileReconcilingMissingPvtData(t *testing.T) {
 	fetcher := &mocks.ReconciliationFetcher{}
 	committer.On("GetMissingPvtDataTracker").Return(nil, errors.New("failed to obtain missing pvt data tracker"))
 
-	r := NewReconciler("", metrics, committer, fetcher,
-		&ReconcilerConfig{SleepInterval: time.Millisecond * 100, BatchSize: 1, IsEnabled: true})
+	r := NewReconciler(
+		"",
+		metrics,
+		committer,
+		fetcher,
+		&PrivdataConfig{
+			ReconcileSleepInterval: time.Millisecond * 100,
+			ReconcileBatchSize:     1,
+			ReconciliationEnabled:  true,
+		})
 	err := r.reconcile()
 	assert.Error(t, err)
 	assert.Contains(t, "failed to obtain missing pvt data tracker", err.Error())
 
 	committer.Mock = mock.Mock{}
 	committer.On("GetMissingPvtDataTracker").Return(nil, nil)
 	r = NewReconciler("", metrics, committer, fetcher,
-		&ReconcilerConfig{SleepInterval: time.Millisecond * 100, BatchSize: 1, IsEnabled: true})
+		&PrivdataConfig{ReconcileSleepInterval: time.Millisecond * 100, ReconcileBatchSize: 1, ReconciliationEnabled: true})
 	err = r.reconcile()
 	assert.Error(t, err)
 	assert.Contains(t, "got nil as MissingPvtDataTracker, exiting...", err.Error())
@@ -472,7 +512,7 @@ func TestFailuresWhileReconcilingMissingPvtData(t *testing.T) {
 	committer.Mock = mock.Mock{}
 	committer.On("GetMissingPvtDataTracker").Return(missingPvtDataTracker, nil)
 	r = NewReconciler("", metrics, committer, fetcher,
-		&ReconcilerConfig{SleepInterval: time.Millisecond * 100, BatchSize: 1, IsEnabled: true})
+		&PrivdataConfig{ReconcileSleepInterval: time.Millisecond * 100, ReconcileBatchSize: 1, ReconciliationEnabled: true})
 	err = r.reconcile()
 	assert.Error(t, err)
 	assert.Contains(t, "failed get missing pvt data for recent blocks", err.Error())

gossip/privdata/util.go

@@ -8,7 +8,6 @@ package privdata
 
 import (
 	"fmt"
-	"time"
 
 	"github.com/golang/protobuf/proto"
 	"github.com/hyperledger/fabric/core/ledger"
@@ -19,7 +18,6 @@ import (
 	"github.com/hyperledger/fabric/protos/ledger/rwset/kvrwset"
 	"github.com/hyperledger/fabric/protos/msp"
 	"github.com/hyperledger/fabric/protos/peer"
-	"github.com/spf13/viper"
 )
 
 type txValidationFlags []uint8
@@ -303,41 +301,3 @@ func (f *digestsAndSourceFactory) toSources(peers ...string) *digestsAndSourceFa
 func (f *digestsAndSourceFactory) create() dig2sources {
 	return f.d2s
 }
-
-const (
-	rreconcileSleepIntervalConfigKey = "peer.gossip.pvtData.reconcileSleepInterval"
-	reconcileSleepIntervalDefault    = time.Minute * 1
-	reconcileBatchSizeConfigKey      = "peer.gossip.pvtData.reconcileBatchSize"
-	reconcileBatchSizeDefault        = 10
-	reconciliationEnabledConfigKey   = "peer.gossip.pvtData.reconciliationEnabled"
-)
-
-// this func reads reconciler configuration values from core.yaml and returns ReconcilerConfig
-func GetReconcilerConfig() *ReconcilerConfig {
-	reconcileSleepInterval := viper.GetDuration(rreconcileSleepIntervalConfigKey)
-	if reconcileSleepInterval == 0 {
-		logger.Warning("Configuration key", rreconcileSleepIntervalConfigKey, "isn't set, defaulting to", reconcileSleepIntervalDefault)
-		reconcileSleepInterval = reconcileSleepIntervalDefault
-	}
-	reconcileBatchSize := viper.GetInt(reconcileBatchSizeConfigKey)
-	if reconcileBatchSize == 0 {
-		logger.Warning("Configuration key", reconcileBatchSizeConfigKey, "isn't set, defaulting to", reconcileBatchSizeDefault)
-		reconcileBatchSize = reconcileBatchSizeDefault
-	}
-	isEnabled := viper.GetBool(reconciliationEnabledConfigKey)
-	return &ReconcilerConfig{SleepInterval: reconcileSleepInterval, BatchSize: reconcileBatchSize, IsEnabled: isEnabled}
-}
-
-const (
-	transientBlockRetentionConfigKey = "peer.gossip.pvtData.transientstoreMaxBlockRetention"
-	TransientBlockRetentionDefault   = 1000
-)
-
-func GetTransientBlockRetention() uint64 {
-	transientBlockRetention := uint64(viper.GetInt(transientBlockRetentionConfigKey))
-	if transientBlockRetention == 0 {
-		logger.Warning("Configuration key", transientBlockRetentionConfigKey, "isn't set, defaulting to", TransientBlockRetentionDefault)
-		transientBlockRetention = TransientBlockRetentionDefault
-	}
-	return transientBlockRetention
-}