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

CouchDB index support for implicit collections #4794

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bkiran6398
Copy link
Contributor

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

  • New feature

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

  • Please review the way of fetching localMspId directly using viper at NewDB constructor function.
  • Added few more tar file entries to represent implicit collection indexes in unit tests. (However UT TestHandleChainCodeDeployOnCouchDB was already failing due to some other error)

Related issues

Signed-off-by: bkiran6398 <bkiran6398@gmail.com>

documentation update to specify couchDB index support for implicit collections

Signed-off-by: bkiran6398 <bkiran6398@gmail.com>

added support for global index creation for all implicit collections

Signed-off-by: bkiran6398 <bkiran6398@gmail.com>

documentation update to brief global implicit collection index creation

Signed-off-by: bkiran6398 <bkiran6398@gmail.com>
@bkiran6398 bkiran6398 requested review from a team as code owners April 4, 2024 08:10
Copy link
Contributor

@denyeart denyeart left a 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
Copy link
Contributor

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().

@denyeart
Copy link
Contributor

We'll want an integration test for both the single org use case and all org use case.
It should be feasible to extend the existing CouchDB index integration test here:
https://github.com/hyperledger/fabric/blob/main/integration/ledger/couchdb_indexes_test.go

@denyeart
Copy link
Contributor

denyeart commented Apr 19, 2024

Not many people use Windows for Fabric, but still, asterisk is not allowed in Windows directory names. Maybe use _implicit_all_orgs?

Signed-off-by: bkiran6398 <bkiran6398@gmail.com>
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.

2 participants