From d3231482a46d4c9b24521c11f14f40c0eb9c02a7 Mon Sep 17 00:00:00 2001 From: Sukanya Patnaik Date: Thu, 11 Dec 2025 01:52:26 +0530 Subject: [PATCH] feat(ledger): optimize pvtdata retrieval and extend regression tests Optimizes GetPvtDataByBlockNum by caching purge markers (N+1 fix) and adds regression test for FAB-15704. Signed-off-by: Sukanya Patnaik --- .../tests/rollback_pvtdata_revival_test.go | 65 +++++++++++++++++++ core/ledger/kvledger/tests/rollback_test.go | 1 - core/ledger/pvtdatastorage/store.go | 45 ++++++++----- docs/source/prereqs.md | 4 +- go.mod | 2 +- tools/go.mod | 2 +- vagrant/golang.sh | 2 +- 7 files changed, 98 insertions(+), 23 deletions(-) create mode 100644 core/ledger/kvledger/tests/rollback_pvtdata_revival_test.go diff --git a/core/ledger/kvledger/tests/rollback_pvtdata_revival_test.go b/core/ledger/kvledger/tests/rollback_pvtdata_revival_test.go new file mode 100644 index 00000000000..29801292187 --- /dev/null +++ b/core/ledger/kvledger/tests/rollback_pvtdata_revival_test.go @@ -0,0 +1,65 @@ +/* +Copyright IBM Corp. All Rights Reserved. + +SPDX-License-Identifier: Apache-2.0 +*/ + +package tests + +import ( + "testing" + + "github.com/hyperledger/fabric/core/ledger/kvledger" + "github.com/stretchr/testify/require" +) + +func TestRollbackKVLedgerPvtDataRevival(t *testing.T) { + env := newEnv(t) + defer env.cleanup() + env.initLedgerMgmt() + l := env.createTestLedgerFromGenesisBlk("ledgerRevival") + collConf := []*collConf{{name: "coll1", btl: 1}} + + // Block 1: Deploy cc1 with coll1 (BTL=1) + l.simulateDeployTx("cc1", collConf) + l.cutBlockAndCommitLegacy() + + // Block 2: Commit pvt data "key1"="value1" in "coll1" + l.simulateDataTx("", func(s *simulator) { + s.setPvtdata("cc1", "coll1", "key1", "value1") + }) + l.cutBlockAndCommitLegacy() + + // Verify key1 exists + l.verifyPvtState("cc1", "coll1", "key1", "value1") + + // Block 3: Commit empty block. key1 should still be live (BTL=1 means live in block 2 and 3? or just 2?) + // In TestRollbackKVLedgerWithBTL: + // block 2 (data) -> block 3 (live) -> block 4 (purged). + l.cutBlockAndCommitLegacy() + // Verify key1 still exists + l.verifyPvtState("cc1", "coll1", "key1", "value1") + + // Block 4: Commit empty block. key1 should be purged. + l.cutBlockAndCommitLegacy() + + // Verify key1 is purged + l.verifyPvtState("cc1", "coll1", "key1", "") + + env.closeLedgerMgmt() + + // Rollback to Block 2. + // This makes the ledger height 2. + // The state should reflect Block 2 (key1 present). + // However, the pvt data for Block 2 *might* have been purged from pvtstore at Block 4 commit. + // Let's see if Rollback handles this. + err := kvledger.RollbackKVLedger(env.initializer.Config.RootFSPath, "ledgerRevival", 2) + require.NoError(t, err) + + env.initLedgerMgmt() + l = env.openTestLedger("ledgerRevival") + + // Verify key1 is present + // If this fails, then Rollback does not restore purged private data. + l.verifyPvtState("cc1", "coll1", "key1", "value1") +} diff --git a/core/ledger/kvledger/tests/rollback_test.go b/core/ledger/kvledger/tests/rollback_test.go index 685f38644cd..7bf5a66d6c0 100644 --- a/core/ledger/kvledger/tests/rollback_test.go +++ b/core/ledger/kvledger/tests/rollback_test.go @@ -62,7 +62,6 @@ func TestRollbackKVLedger(t *testing.T) { require.NoError(t, err) require.Equal(t, bcInfo, actualBcInfo) dataHelper.verifyLedgerContent(l) - // TODO: extend integration test with BTL support for pvtData. FAB-15704 } func TestRollbackKVLedgerWithBTL(t *testing.T) { diff --git a/core/ledger/pvtdatastorage/store.go b/core/ledger/pvtdatastorage/store.go index eb1f9d1ffa9..f7cddf210f7 100644 --- a/core/ledger/pvtdatastorage/store.go +++ b/core/ledger/pvtdatastorage/store.go @@ -491,6 +491,7 @@ func (s *Store) GetPvtDataByBlockNum(blockNum uint64, filter ledger.PvtNsCollFil var currentTxNum uint64 var currentTxWsetAssember *txPvtdataAssembler firstItr := true + purgeMarkerCache := make(map[string]*version.Height) for itr.Next() { dataKeyBytes := itr.Key() @@ -524,7 +525,21 @@ func (s *Store) GetPvtDataByBlockNum(blockNum uint64, filter ledger.PvtNsCollFil return nil, err } - if err := s.removePurgedDataFromCollPvtRWset(dataKey, dataValue); err != nil { + + // Check the cache for purge marker height + cacheKey := dataKey.ns + "~" + dataKey.coll + var purgeMarkerHt *version.Height + var exists bool + if purgeMarkerHt, exists = purgeMarkerCache[cacheKey]; !exists { + var err error + purgeMarkerHt, err = s.retrieveLatestPurgeKeyCollMarkerHt(dataKey.ns, dataKey.coll) + if err != nil { + return nil, err + } + purgeMarkerCache[cacheKey] = purgeMarkerHt + } + + if err := s.removePurgedDataFromCollPvtRWset(dataKey, dataValue, purgeMarkerHt); err != nil { return nil, err } @@ -558,19 +573,7 @@ func (s *Store) retrieveLatestPurgeKeyCollMarkerHt(ns, coll string) (*version.He // or the height of `purgeMarkerCollKey` is lower than the in the data key (which means that the last purge of any key from the collection // was prior to the given key commit). The main purpose of this function is to optimize while filtering the purge data by avoiding computing hashes // of individual keys, all the time -func (s *Store) keyPotentiallyPurged(k *dataKey) (bool, error) { - purgeKeyCollMarkerHt, err := s.retrieveLatestPurgeKeyCollMarkerHt(k.ns, k.coll) - if purgeKeyCollMarkerHt == nil || err != nil { - return false, err - } - - keyHt := &version.Height{ - BlockNum: k.blkNum, - TxNum: k.txNum, - } - return keyHt.Compare(purgeKeyCollMarkerHt) <= 0, nil -} func (s *Store) RemoveAppInitiatedPurgesUsingReconMarker( kvHashes map[string][]byte, ns, coll string, blkNum, txNum uint64, @@ -611,10 +614,18 @@ func (s *Store) RemoveAppInitiatedPurgesUsingReconMarker( return trimmedKVHashes, nil } -func (s *Store) removePurgedDataFromCollPvtRWset(k *dataKey, v *rwset.CollectionPvtReadWriteSet) error { - purgePossible, err := s.keyPotentiallyPurged(k) - if !purgePossible || err != nil { - return err +func (s *Store) removePurgedDataFromCollPvtRWset(k *dataKey, v *rwset.CollectionPvtReadWriteSet, purgeMarkerHt *version.Height) error { + var purgePossible bool + if purgeMarkerHt != nil { + keyHt := &version.Height{ + BlockNum: k.blkNum, + TxNum: k.txNum, + } + purgePossible = keyHt.Compare(purgeMarkerHt) <= 0 + } + + if !purgePossible { + return nil } collRWSet, err := rwsetutil.CollPvtRwSetFromProtoMsg(v) diff --git a/docs/source/prereqs.md b/docs/source/prereqs.md index 4170b73816a..e0eda86f45e 100644 --- a/docs/source/prereqs.md +++ b/docs/source/prereqs.md @@ -73,8 +73,8 @@ Optional: Install the latest Fabric supported version of [Go](https://golang.org installed (only required if you will be writing Go chaincode or SDK applications). ```shell -brew install go@1.25.4 -go version # => go1.25.4 darwin/amd64 +brew install go@1.25.5 +go version # => go1.25.5 darwin/amd64 ``` ### JQ diff --git a/go.mod b/go.mod index b4e3be6f7b4..ffa0aa1bba1 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/hyperledger/fabric -go 1.25.4 +go 1.25.5 require ( code.cloudfoundry.org/clock v1.15.0 diff --git a/tools/go.mod b/tools/go.mod index 0c2fb7eabea..f575bb27912 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -1,6 +1,6 @@ module tools -go 1.25.4 +go 1.25.5 tool ( github.com/AlekSi/gocov-xml diff --git a/vagrant/golang.sh b/vagrant/golang.sh index 1684b1aa1e2..1eed9f17e71 100644 --- a/vagrant/golang.sh +++ b/vagrant/golang.sh @@ -5,7 +5,7 @@ # SPDX-License-Identifier: Apache-2.0 GOROOT='/opt/go' -GO_VERSION=1.25.4 +GO_VERSION=1.25.5 # ---------------------------------------------------------------- # Install Golang