[FAB-11334] Scrubs partially constructed/deleted ledgers at peer init #2754
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
manish-sethi
approved these changes
Jul 14, 2021
Comment on lines
+17
to
+40
| 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) | ||
| } |
There was a problem hiding this comment.
Though, the extra test does not harm and in this particular case it's very small enough addition that could easily be ignored. However, in general we should prefer to be thoughtful of the purpose of an additional test.
- It's hard to think of a bug/issue which would make this test pass while the fail the next one.
- Neither does this test tests a separate section of the code.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Peer init will now remove any ledger with status
UNDER_CONSTRUCTIONorUNDER_DELETIONat startup.
Signed-off-by: Josh Kneubuhl jkneubuh@us.ibm.com
Type of change
Description
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
at startup.
Additional details
Related issues