[FAB-11334] Scrubs partially constructed/deleted ledgers at peer init (…

…#2754)

Unjoining a peer from a channel will mark the ledger with status UNDER_DELETION
and proceed with the ledger removal.  If the unjoin operation fails, the ledger
will include residue from the partial deletion, leaving the kvledger in a
questionable state.  This commit forces the peer initialization to scan for any
ledgers with an UNDER_DELETION status, removing partial ledgers from the peer.

Signed-off-by: Josh Kneubuhl <jkneubuh@us.ibm.com>

core/ledger/kvledger/delete_partial_ledgers_test.go

@@ -0,0 +1,117 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package kvledger
+
+import (
+	"testing"
+
+	"github.com/hyperledger/fabric/core/ledger/kvledger/msgs"
+	"github.com/hyperledger/fabric/core/ledger/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDeleteUnderDeletionLedger(t *testing.T) {
+	conf, cleanup := testConfig(t)
+	conf.HistoryDBConfig.Enabled = true
+	defer cleanup()
+
+	provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{})
+	defer provider.Close()
+
+	// Set up a ledger and mark it as "under deletion."  This emulates the
+	// state when a peer was in the process of unjoining a channel and the
+	// unjoin operation crashed mid delete.
+	ledgerID := constructTestLedger(t, provider, 0)
+	verifyLedgerIDExists(t, provider, ledgerID, msgs.Status_ACTIVE)
+
+	// doom the newly created ledger
+	require.NoError(t, provider.idStore.updateLedgerStatus(ledgerID, msgs.Status_UNDER_DELETION))
+	verifyLedgerIDExists(t, provider, ledgerID, msgs.Status_UNDER_DELETION)
+
+	// explicitly call the delete on the provider.
+	provider.deletePartialLedgers()
+
+	// goodbye, ledger.
+	verifyLedgerDoesNotExist(t, provider, ledgerID)
+}
+
+func TestDeletePartialLedgers(t *testing.T) {
+	conf, cleanup := testConfig(t)
+	conf.HistoryDBConfig.Enabled = true
+	defer cleanup()
+
+	provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{})
+	defer provider.Close()
+
+	targetStatus := []msgs.Status{
+		msgs.Status_ACTIVE,
+		msgs.Status_UNDER_DELETION,
+		msgs.Status_ACTIVE,
+		msgs.Status_UNDER_DELETION,
+		msgs.Status_UNDER_CONSTRUCTION,
+	}
+
+	constructPartialLedgers(t, provider, targetStatus)
+
+	// delete the under deletion ledgers
+	err := provider.deletePartialLedgers()
+	require.NoError(t, err)
+
+	verifyPartialLedgers(t, provider, targetStatus)
+}
+
+func TestNewProviderDeletesPartialLedgers(t *testing.T) {
+	conf, cleanup := testConfig(t)
+	conf.HistoryDBConfig.Enabled = true
+	defer cleanup()
+
+	provider := testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{})
+
+	targetStatus := []msgs.Status{
+		msgs.Status_ACTIVE,
+		msgs.Status_UNDER_DELETION,
+		msgs.Status_UNDER_CONSTRUCTION,
+		msgs.Status_ACTIVE,
+		msgs.Status_UNDER_DELETION,
+	}
+
+	constructPartialLedgers(t, provider, targetStatus)
+
+	// Close and re-open the provider.  The initialization should have scrubbed the partial ledgers.
+	provider.Close()
+	provider = testutilNewProvider(conf, t, &mock.DeployedChaincodeInfoProvider{})
+	defer provider.Close()
+
+	verifyPartialLedgers(t, provider, targetStatus)
+}
+
+// Construct a series of test ledgers, each with a target status.
+func constructPartialLedgers(t *testing.T, provider *Provider, targetStatus []msgs.Status) {
+	for i := 0; i < len(targetStatus); i++ {
+		ledgerID := constructTestLedger(t, provider, i)
+		require.NoError(t, provider.idStore.updateLedgerStatus(ledgerID, targetStatus[i]))
+		verifyLedgerIDExists(t, provider, ledgerID, targetStatus[i])
+	}
+}
+
+// Check that all UNDER_CONSTRUCTION and UNDER_DELETION ledgers were scrubbed.
+func verifyPartialLedgers(t *testing.T, provider *Provider, targetStatus []msgs.Status) {
+	// Also double-check that deleted ledgers do not appear in the provider listing.
+	activeLedgers, err := provider.List()
+	require.NoError(t, err)
+
+	for i := 0; i < len(targetStatus); i++ {
+		ledgerID := constructTestLedgerID(i)
+		if targetStatus[i] == msgs.Status_UNDER_CONSTRUCTION || targetStatus[i] == msgs.Status_UNDER_DELETION {
+			verifyLedgerDoesNotExist(t, provider, ledgerID)
+			require.NotContains(t, ledgerID, activeLedgers)
+		} else {
+			verifyLedgerIDExists(t, provider, ledgerID, targetStatus[i])
+			require.Contains(t, activeLedgers, ledgerID)
+		}
+	}
+}

core/ledger/kvledger/kv_ledger_provider.go

@@ -119,7 +119,7 @@ func NewProvider(initializer *ledger.Initializer) (pr *Provider, e error) {
 		return nil, err
 	}
 	p.initLedgerStatistics()
-	if err := p.deleteUnderConstructionLedgers(); err != nil {
+	if err := p.deletePartialLedgers(); err != nil {
 		return nil, err
 	}
 	if err := p.initSnapshotDir(); err != nil {
@@ -420,21 +420,21 @@ func (p *Provider) Close() {
 	}
 }
 
-// deleteUnderConstructionLedgers checks whether there is a ledger that is partially created - this would be the case
-// if a crash had happened during creation of ledger and the ledger creation could have been left in intermediate
-// state. This function deletes such a ledger so the ledger can be recreated a fresh
-func (p *Provider) deleteUnderConstructionLedgers() error {
-	logger.Debugf("Recovering under construction ledger")
+// deletePartialLedgers scans for and deletes any ledger with a status of UNDER_CONSTRUCTION or UNDER_DELETION.
+// UNDER_CONSTRUCTION ledgers represent residual structures created as a side effect of a crash during ledger creation.
+// UNDER_DELETION ledgers represent residual structures created as a side effect of a crash during a peer channel unjoin.
+func (p *Provider) deletePartialLedgers() error {
+	logger.Debug("Removing ledgers in state UNDER_CONSTRUCTION or UNDER_DELETION")
 	itr := p.idStore.db.GetIterator(metadataKeyPrefix, metadataKeyStop)
 	defer itr.Release()
 	if err := itr.Error(); err != nil {
-		return errors.WithMessage(err, "error while obtaining iterator for checking for any under construction ledger")
+		return errors.WithMessage(err, "error obtaining iterator for incomplete ledger scans")
 	}
 	for {
 		hasMore := itr.Next()
 		err := itr.Error()
 		if err != nil {
-			return errors.WithMessage(err, "error while iterating over ledger for checking for any under construction ledger")
+			return errors.WithMessage(err, "error while iterating over ledger list while scanning for incomplete ledgers")
 		}
 		if !hasMore {
 			return nil
@@ -444,18 +444,20 @@ func (p *Provider) deleteUnderConstructionLedgers() error {
 		if err := proto.Unmarshal(itr.Value(), metadata); err != nil {
 			return errors.Wrapf(err, "error while unmarshalling metadata bytes for ledger [%s]", ledgerID)
 		}
-		if metadata.Status == msgs.Status_UNDER_CONSTRUCTION {
+		if metadata.Status == msgs.Status_UNDER_CONSTRUCTION || metadata.Status == msgs.Status_UNDER_DELETION {
 			logger.Infow(
-				"An under-construction ledger found at start. This indicates a peer stop/crash during creation of a ledger. Going to delete the partially created ledger",
+				"A partial ledger was identified at peer launch, indicating a peer stop/crash during creation or a failed channel unjoin.  The partial ledger wil be deleted.",
 				"ledgerID", ledgerID,
+				"Status", metadata.Status,
 			)
 			if err := p.runCleanup(ledgerID); err != nil {
 				logger.Errorw(
 					"Error while deleting a partially created ledger at start",
 					"ledgerID", ledgerID,
+					"Status", metadata.Status,
 					"error", err,
 				)
-				return errors.WithMessagef(err, "error while deleting a partially created ledger at start for ledger = [%s]", ledgerID)
+				return errors.WithMessagef(err, "error while deleting a partially constructed ledger with status [%s] at start for ledger = [%s]", metadata.Status, ledgerID)
 			}
 		}
 	}

core/ledger/kvledger/kv_ledger_provider_test.go

@@ -566,6 +566,19 @@ func constructTestLedgerID(i int) string {
 	return fmt.Sprintf("ledger_%06d", i)
 }
 
+func constructTestLedger(t *testing.T, provider *Provider, sequenceID int) string {
+	ledgerID := constructTestLedgerID(sequenceID)
+	gb, err := configtxtest.MakeGenesisBlock(ledgerID)
+	require.NoError(t, err)
+	require.NotNil(t, gb)
+
+	lgr, err := provider.CreateFromGenesisBlock(gb)
+	require.NoError(t, err)
+	require.NotNil(t, lgr)
+
+	return ledgerID
+}
+
 func testConfig(t *testing.T) (conf *ledger.Config, cleanup func()) {
 	path, err := ioutil.TempDir("", "kvledger")
 	require.NoError(t, err, "Failed to create test ledger directory")