-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
CouchDB index support for implicit collections #4794
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: bkiran6398 <[email protected]> documentation update to specify couchDB index support for implicit collections Signed-off-by: bkiran6398 <[email protected]> added support for global index creation for all implicit collections Signed-off-by: bkiran6398 <[email protected]> documentation update to brief global implicit collection index creation Signed-off-by: bkiran6398 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay but I wanted to actually test it myself since there was no integration test here. This is an overall good approach, thank you for filling this gap!
In my test I found that the implicit collection wildcard approach causes an error at deploy time:
Error: error getting chaincode bytes: walk failed: metadata file path or name is not supported: META-INF/statedb/couchdb/collections/_implicit_org_*/indexes/
This is because of this validation that doesn't allow asterisk:
https://github.com/hyperledger/fabric/blob/main/internal/ccmetadata/validators.go#L31
} | ||
|
||
// NewDB wraps a VersionedDB instance. The public data is managed directly by the wrapped versionedDB. | ||
// For managing the hashed data and private data, this implementation creates separate namespaces in the wrapped db | ||
func NewDB(vdb statedb.VersionedDB, ledgerid string, metadataHint *metadataHint) (*DB, error) { | ||
return &DB{vdb, metadataHint}, nil | ||
return &DB{vdb, metadataHint, viper.GetString("peer.localMspId")}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the config from within the implementation is considered a bad practice. We use the dependency injection pattern to pass down dependencies including peer configuration.
In this case, in kv_ledger_provider.go when calling privacyenabledstate.NewDBProvider() you can pass the existing
p.initializer.MembershipInfoProvider.MyImplicitCollectionName()
.
We'll want an integration test for both the single org use case and all org use case. |
Not many people use Windows for Fabric, but still, asterisk is not allowed in Windows directory names. Maybe use |
Signed-off-by: bkiran6398 <[email protected]>
Allow CouchDB index creation on implicit collection of organisations.
Indexes can be created for specific organisation's implicit collections at:
META-INF/statedb/couchdb/collections/_implicit_org_<MSP_ID>
directory of the chaincode package.Common indexes for all the organisations' implicit collection can be created at:
META-INF/statedb/couchdb/collections/_implicit_org_*
directory of the chaincode package.(
*
acts as wildcard for all organisation MSP ids. )Type of change
Description
Updated chaincode deployment handler to check if the indexes are for implicit collection if given collection directory name not explicitly defined in collection config. If the implicit collection indexes belong this handling peer's organisation or passed as global implicit collection indexes (using
*
notation), then indexes are created on corresponding implicit collection.Additional details
localMspId
directly usingviper
atNewDB
constructor function.TestHandleChainCodeDeployOnCouchDB
was already failing due to some other error)Related issues