From 6bf0542edb5bc87f2da4a9d7dee1c19f8a48e311 Mon Sep 17 00:00:00 2001 From: Senthil Nathan N Date: Thu, 17 Sep 2020 19:36:44 +0530 Subject: [PATCH] address review comments (#1890) Signed-off-by: Senthil Nathan N --- .../tests/missing_pvtdata_retrieval_test.go | 16 +- core/ledger/pvtdatastorage/store_test.go | 142 ++++++++++++------ internal/peer/node/config.go | 2 - internal/peer/node/config_test.go | 4 +- 4 files changed, 97 insertions(+), 67 deletions(-) diff --git a/core/ledger/kvledger/tests/missing_pvtdata_retrieval_test.go b/core/ledger/kvledger/tests/missing_pvtdata_retrieval_test.go index 3425535e741..49ca6fc2e74 100644 --- a/core/ledger/kvledger/tests/missing_pvtdata_retrieval_test.go +++ b/core/ledger/kvledger/tests/missing_pvtdata_retrieval_test.go @@ -73,11 +73,6 @@ func TestGetMissingPvtData(t *testing.T) { blk, expectedMissingPvtDataInfo := setup(h) - // verify missing pvtdata info - require.Equal(t, uint64(2), blk.Block.Header.Number) - h.verifyBlockAndPvtDataSameAs(2, blk) - h.verifyMissingPvtDataSameAs(int(2), expectedMissingPvtDataInfo) - // commit block 3 h.simulateDataTx("", func(s *simulator) { s.setPvtdata("cc1", "coll1", "key4", "value4") @@ -145,12 +140,7 @@ func TestGetMissingPvtData(t *testing.T) { env.initLedgerMgmt() h := env.newTestHelperCreateLgr("ledger1", t) - blk, expectedMissingPvtDataInfo := setup(h) - - // verify missing pvtdata info - require.Equal(t, uint64(2), blk.Block.Header.Number) - h.verifyBlockAndPvtDataSameAs(2, blk) - h.verifyMissingPvtDataSameAs(int(2), expectedMissingPvtDataInfo) + _, expectedMissingPvtDataInfo := setup(h) h.commitPvtDataOfOldBlocks(nil, expectedMissingPvtDataInfo) for i := 0; i < 5; i++ { @@ -163,7 +153,7 @@ func TestGetMissingPvtData(t *testing.T) { h = env.newTestHelperOpenLgr("ledger1", t) for i := 0; i < 5; i++ { - h.verifyMissingPvtDataSameAs(int(2), expectedMissingPvtDataInfo) + h.verifyMissingPvtDataSameAs(2, expectedMissingPvtDataInfo) } env.closeLedgerMgmt() @@ -172,7 +162,7 @@ func TestGetMissingPvtData(t *testing.T) { h = env.newTestHelperOpenLgr("ledger1", t) for i := 0; i < 5; i++ { - h.verifyMissingPvtDataSameAs(int(2), ledger.MissingPvtDataInfo{}) + h.verifyMissingPvtDataSameAs(2, ledger.MissingPvtDataInfo{}) } }) } diff --git a/core/ledger/pvtdatastorage/store_test.go b/core/ledger/pvtdatastorage/store_test.go index b3e7dcb0d31..847c86a89f0 100644 --- a/core/ledger/pvtdatastorage/store_test.go +++ b/core/ledger/pvtdatastorage/store_test.go @@ -192,67 +192,109 @@ func TestStoreIteratorError(t *testing.T) { } func TestGetMissingDataInfo(t *testing.T) { - ledgerid := "TestGetMissingDataInfoFromPrioDeprioList" - btlPolicy := btltestutil.SampleBTLPolicy( - map[[2]string]uint64{ - {"ns-1", "coll-1"}: 0, - {"ns-1", "coll-2"}: 0, - }, - ) - env := NewTestStoreEnv(t, ledgerid, btlPolicy, pvtDataConf()) - defer env.Cleanup() - store := env.TestStore - - // construct missing data for block 1 - blk1MissingData := make(ledger.TxMissingPvtData) - blk1MissingData.Add(1, "ns-1", "coll-1", true) - blk1MissingData.Add(1, "ns-1", "coll-2", true) - - require.NoError(t, store.Commit(0, nil, nil)) - require.NoError(t, store.Commit(1, nil, blk1MissingData)) - - deprioritizedList := ledger.MissingPvtDataInfo{ - 1: ledger.MissingBlockPvtdataInfo{ - 1: { - { - Namespace: "ns-1", - Collection: "coll-2", - }, + setup := func(ledgerid string, c *PrivateDataConfig) *Store { + btlPolicy := btltestutil.SampleBTLPolicy( + map[[2]string]uint64{ + {"ns-1", "coll-1"}: 0, + {"ns-1", "coll-2"}: 0, }, - }, - } - require.NoError(t, store.CommitPvtDataOfOldBlocks(nil, deprioritizedList)) + ) - expectedPrioMissingDataInfo := ledger.MissingPvtDataInfo{ - 1: ledger.MissingBlockPvtdataInfo{ - 1: { - { - Namespace: "ns-1", - Collection: "coll-1", + env := NewTestStoreEnv(t, ledgerid, btlPolicy, c) + t.Cleanup( + func() { + defer env.Cleanup() + }, + ) + store := env.TestStore + + // construct missing data for block 1 + blk1MissingData := make(ledger.TxMissingPvtData) + blk1MissingData.Add(1, "ns-1", "coll-1", true) + blk1MissingData.Add(1, "ns-1", "coll-2", true) + + require.NoError(t, store.Commit(0, nil, nil)) + require.NoError(t, store.Commit(1, nil, blk1MissingData)) + + deprioritizedList := ledger.MissingPvtDataInfo{ + 1: ledger.MissingBlockPvtdataInfo{ + 1: { + { + Namespace: "ns-1", + Collection: "coll-2", + }, }, }, - }, + } + require.NoError(t, store.CommitPvtDataOfOldBlocks(nil, deprioritizedList)) + + return env.TestStore } - expectedDeprioMissingDataInfo := ledger.MissingPvtDataInfo{ - 1: ledger.MissingBlockPvtdataInfo{ - 1: { - { - Namespace: "ns-1", - Collection: "coll-2", + t.Run("always access deprioritized missing data", func(t *testing.T) { + conf := pvtDataConf() + conf.DeprioritizedDataReconcilerInterval = 0 + store := setup("testGetMissingDataInfoFromDeprioList", conf) + + expectedDeprioMissingDataInfo := ledger.MissingPvtDataInfo{ + 1: ledger.MissingBlockPvtdataInfo{ + 1: { + { + Namespace: "ns-1", + Collection: "coll-2", + }, }, }, - }, - } + } - assertMissingDataInfo(t, store, expectedPrioMissingDataInfo, 2) + for i := 0; i < 2; i++ { + assertMissingDataInfo(t, store, expectedDeprioMissingDataInfo, 2) + } + }) - store.accessDeprioMissingDataAfter = time.Now() - expectedNextAccessDeprioMissingDataTime := time.Now().Add(store.deprioritizedDataReconcilerInterval) - assertMissingDataInfo(t, store, expectedDeprioMissingDataInfo, 2) + t.Run("change the deprioritized missing data access time", func(t *testing.T) { + conf := pvtDataConf() + conf.DeprioritizedDataReconcilerInterval = 300 * time.Minute + store := setup("testGetMissingDataInfoFromPrioAndDeprioList", conf) + + expectedPrioMissingDataInfo := ledger.MissingPvtDataInfo{ + 1: ledger.MissingBlockPvtdataInfo{ + 1: { + { + Namespace: "ns-1", + Collection: "coll-1", + }, + }, + }, + } + + expectedDeprioMissingDataInfo := ledger.MissingPvtDataInfo{ + 1: ledger.MissingBlockPvtdataInfo{ + 1: { + { + Namespace: "ns-1", + Collection: "coll-2", + }, + }, + }, + } + + for i := 0; i < 3; i++ { + assertMissingDataInfo(t, store, expectedPrioMissingDataInfo, 2) + } + + store.accessDeprioMissingDataAfter = time.Now().Add(-time.Second) + lesserThanNextAccessTime := time.Now().Add(store.deprioritizedDataReconcilerInterval).Add(-2 * time.Second) + greaterThanNextAccessTime := time.Now().Add(store.deprioritizedDataReconcilerInterval).Add(2 * time.Second) + assertMissingDataInfo(t, store, expectedDeprioMissingDataInfo, 2) + + require.True(t, store.accessDeprioMissingDataAfter.After(lesserThanNextAccessTime)) + require.False(t, store.accessDeprioMissingDataAfter.After(greaterThanNextAccessTime)) + for i := 0; i < 3; i++ { + assertMissingDataInfo(t, store, expectedPrioMissingDataInfo, 2) + } + }) - require.True(t, store.accessDeprioMissingDataAfter.After(expectedNextAccessDeprioMissingDataTime)) - assertMissingDataInfo(t, store, expectedPrioMissingDataInfo, 2) } func TestExpiryDataNotIncluded(t *testing.T) { diff --git a/internal/peer/node/config.go b/internal/peer/node/config.go index 09d141d4323..4a76b4f9a65 100644 --- a/internal/peer/node/config.go +++ b/internal/peer/node/config.go @@ -7,7 +7,6 @@ SPDX-License-Identifier: Apache-2.0 package node import ( - "fmt" "path/filepath" "time" @@ -45,7 +44,6 @@ func ledgerConfig() *ledger.Config { deprioritizedDataReconcilerInterval := 60 * time.Minute if viper.IsSet("ledger.pvtdataStore.deprioritizedDataReconcilerInterval") { deprioritizedDataReconcilerInterval = viper.GetDuration("ledger.pvtdataStore.deprioritizedDataReconcilerInterval") - fmt.Println(deprioritizedDataReconcilerInterval) } rootFSPath := filepath.Join(coreconfig.GetPath("peer.fileSystemPath"), "ledgersData") diff --git a/internal/peer/node/config_test.go b/internal/peer/node/config_test.go index a8070c77c89..171c569611f 100644 --- a/internal/peer/node/config_test.go +++ b/internal/peer/node/config_test.go @@ -114,7 +114,7 @@ func TestLedgerConfig(t *testing.T) { "ledger.pvtdataStore.collElgProcMaxDbBatchSize": 50000, "ledger.pvtdataStore.collElgProcDbBatchesInterval": 10000, "ledger.pvtdataStore.purgeInterval": 1000, - "ledger.pvtdataStore.deprioritizedDataReconcilerInterval": "60m", + "ledger.pvtdataStore.deprioritizedDataReconcilerInterval": "180m", "ledger.history.enableHistoryDatabase": true, "ledger.snapshots.rootDir": "/peerfs/snapshots", }, @@ -141,7 +141,7 @@ func TestLedgerConfig(t *testing.T) { MaxBatchSize: 50000, BatchesInterval: 10000, PurgeInterval: 1000, - DeprioritizedDataReconcilerInterval: 60 * time.Minute, + DeprioritizedDataReconcilerInterval: 180 * time.Minute, }, HistoryDBConfig: &ledger.HistoryDBConfig{ Enabled: true,