Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FAB-17992] Remove ledger blockstore data for a channel #1423

Merged
merged 1 commit into from
Jun 26, 2020

Conversation

wenjianqiao
Copy link
Contributor

@wenjianqiao wenjianqiao commented Jun 19, 2020

Signed-off-by: Wenjian Qiao wenjianq@gmail.com

Type of change

  • New feature

Description

Add a Remove function to block store provider to remove block index
and blocks directory for a channel. It will be called by the orderer
and peer as part of channel deletion.

Additional details

This PR adds the remove function to remove the block indexes and blocks dir for the channel. It is expected that the higher level above block store to handle failure recovery.

The PR is based on the current BlockStoreProvider design.

  • no concurrent calls to BlockStoreProvider
  • BlockStoreProvider is not thread-safe, the caller manages thread-safe
  • only one ledger instance is created/opened for a particular ledger, all the time.

The opened BlockStore for the removed channel should not be used any more because it will return invalid data. If needed, we will close the blockstore and make it unusable in a separate PR.

Related issues

Story: https://jira.hyperledger.org/browse/FAB-17992
Epic: https://jira.hyperledger.org/browse/FAB-17712

Copy link
Contributor

@cendhu cendhu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Wenjian, for changing the approach as discussed. While the approach looks good to me, I have a few other suggestions related to improving the code robustness, test, and code readability.

common/ledger/blkstorage/blockstore_provider.go Outdated Show resolved Hide resolved
common/ledger/blkstorage/blockstore_provider_test.go Outdated Show resolved Hide resolved
common/ledger/blkstorage/blockstore_provider_test.go Outdated Show resolved Hide resolved
common/ledger/util/leveldbhelper/leveldb_provider_test.go Outdated Show resolved Hide resolved
@wenjianqiao wenjianqiao force-pushed the fab17992 branch 4 times, most recently from 9e71074 to 1fb4e29 Compare June 25, 2020 18:06
@wenjianqiao wenjianqiao reopened this Jun 25, 2020
@wenjianqiao
Copy link
Contributor Author

@cendhu Thank you. I have addressed your comments.

Add a Remove function to block store provider to remove block index
and blocks directory for a channel. It will be called by the orderer
and peer as part of channel deletion.

Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>
if err := dbHandle.DeleteAll(); err != nil {
return err
}
dbHandle.Close()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit. Can be deferred after getting the handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dbHandle.Close() removes dbHandle from provider's dbHandles map, so it is only called after DeleteAll is successful.

@wenjianqiao
Copy link
Contributor Author

Got intermittent failure and opened FAB-18030.

2020-06-26T12:40:02.0392904Z Waiting for: 2020-06-26T12:40:02.0393318Z Removing member: Endpoint: 127.0.0.1:21514�[0m 2020-06-26T12:40:02.0393516Z 2020-06-26T12:40:02.0393800Z /home/vsts/work/1/go/src/github.com/hyperledger/fabric/integration/gossip/gossip_test.go:343

@cendhu cendhu merged commit 02f0569 into hyperledger:master Jun 26, 2020
sijocherian pushed a commit to sijocherian/fabric that referenced this pull request Jun 28, 2020
…1423)

Add a Remove function to block store provider to remove block index
and blocks directory for a channel. It will be called by the orderer
and peer as part of channel deletion.

Signed-off-by: Wenjian Qiao <wenjianq@gmail.com>
@wenjianqiao wenjianqiao deleted the fab17992 branch July 15, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants