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

INDY-1205: Use RocksDB as a key-value storage #561

Merged
merged 31 commits into from
Mar 21, 2018

Conversation

sergey-shilov
Copy link
Contributor

No description provided.

@ghost
Copy link

ghost commented Mar 6, 2018

Could one of the committers please verify this patch?

@@ -59,7 +60,7 @@
clientBootStrategy = ClientBootStrategy.PoolTxn

hashStore = {
"type": HS_FILE
"type": HS_LEVELDB
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please use rocksdb here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, thanks)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seriously speaking, I've made temporary roll back to leveldb as rocksdb building procedure is not ready yet.

logger = getlogger()


class RocksDbHashStore(HashStore):
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation looks exactly the same as LevelDbHashStore. Maybe use just one implementation with Dependency Injection of a proper key-value implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I propose to merge hash stores implementations after rocksdb build procedure is ready.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -940,7 +932,7 @@ def onStopping(self):

def closeAllKVStores(self):
# Clear leveldb lock files
logger.debug("{} closing level dbs".format(self), extra={"cli": False})
logger.debug("{} closing level/rocks dbs".format(self), extra={"cli": False})
Copy link
Contributor

Choose a reason for hiding this comment

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

A better comment: closing key-value storages

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.



@pytest.yield_fixture(scope="module")
def rocksdbHashStore(tdir):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there the same tests as for LevelDbHashStore? maybe just use the same test with parametrized fixtures?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


def close(self):
raise NotImplementedError
del self._db
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we call del here?

Signed-off-by: Sergey Shilov <[email protected]>
@sergey-shilov sergey-shilov force-pushed the feature/INDY-1205 branch 3 times, most recently from 7bfc05e to f45ab67 Compare March 7, 2018 10:01
@sergey-shilov
Copy link
Contributor Author

Test this please.

ashcherbakov
ashcherbakov previously approved these changes Mar 7, 2018
@sergey-shilov
Copy link
Contributor Author

Test this please.

opts.comparator = IntegerComparator()
self._db = rocksdb.DB(self._db_path, opts)

def get_equal_or_prev(self, key):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we implement it in KeyValueStorageRocksdb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it is correct just in case of integer keys with corresponding comparator. Otherwise the order is lexicographical where '2' is greater than '100', so get_equal_or_prev() does not make sense.

@ashcherbakov ashcherbakov merged commit b656455 into hyperledger:master Mar 21, 2018
lampkin-diet pushed a commit to lampkin-diet/indy-plenum that referenced this pull request Apr 9, 2018
* INDY-1205: Add rocksdb as a key-value backend.

Signed-off-by: Sergey Shilov <[email protected]>

* Add tests for rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Add rocksdb support.

Signed-off-by: Sergey Shilov <[email protected]>

* Use rocksdb as a backend storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix initialisation of rocksdb kvstore.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix test_kv_rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix comparator of KeyValueStorageRocksdbIntKeys class.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix test_state_rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Add unified config-based creation of hash store.

Signed-off-by: Sergey Shilov <[email protected]>

* Change default hash storage from file to rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Integrate rocksdb into state tests.

Signed-off-by: Sergey Shilov <[email protected]>

* Merge kv storages tests into single module.

Signed-off-by: Sergey Shilov <[email protected]>

* Temporary rollback to leveldb.

Signed-off-by: Sergey Shilov <[email protected]>

* Re-factor tests.

Signed-off-by: Sergey Shilov <[email protected]>

* Implement the first version of installation of rocksdb and python-rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Merge leveldb and rocksdb hash storages implementations into single storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Use RocksDB as a key-value storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Tempoprary use leveldb as a default storage for the ledger.

Signed-off-by: Sergey Shilov <[email protected]>

* Adopt getAllTxn() for working with rocksdb iterator.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix db_path property for leveldb and rocksdb, fix test.

Signed-off-by: Sergey Shilov <[email protected]>

* Add build procedure for python-rocksdb and setuptool, use librocksdb deb from sovrin.

Signed-off-by: Sergey Shilov <[email protected]>

* Add missed libs to docker file.

Signed-off-by: Sergey Shilov <[email protected]>

* Change rocksdb package.

Signed-off-by: Sergey Shilov <[email protected]>

* Change rocksdb package for 3d parties build.

Signed-off-by: Sergey Shilov <[email protected]>

* Implement get_equal_or_prev() functionality for KeyValueStorageRocksdbIntKeys.

Signed-off-by: Sergey Shilov <[email protected]>

* Add a helper for init of k/v storage with int keys.

Signed-off-by: Sergey Shilov <[email protected]>

* Add rocksdb tests for the equal-or-prev functionality.

Signed-off-by: Sergey Shilov <[email protected]>

* Fallback to leveldb as we do not want to migrate to rocksdb right now.

Signed-off-by: Sergey Shilov <[email protected]>
lampkin-diet pushed a commit to lampkin-diet/indy-plenum that referenced this pull request Apr 10, 2018
* INDY-1205: Add rocksdb as a key-value backend.

Signed-off-by: Sergey Shilov <[email protected]>

* Add tests for rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Add rocksdb support.

Signed-off-by: Sergey Shilov <[email protected]>

* Use rocksdb as a backend storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix initialisation of rocksdb kvstore.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix test_kv_rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix comparator of KeyValueStorageRocksdbIntKeys class.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix test_state_rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Add unified config-based creation of hash store.

Signed-off-by: Sergey Shilov <[email protected]>

* Change default hash storage from file to rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Integrate rocksdb into state tests.

Signed-off-by: Sergey Shilov <[email protected]>

* Merge kv storages tests into single module.

Signed-off-by: Sergey Shilov <[email protected]>

* Temporary rollback to leveldb.

Signed-off-by: Sergey Shilov <[email protected]>

* Re-factor tests.

Signed-off-by: Sergey Shilov <[email protected]>

* Implement the first version of installation of rocksdb and python-rocksdb.

Signed-off-by: Sergey Shilov <[email protected]>

* Merge leveldb and rocksdb hash storages implementations into single storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Use RocksDB as a key-value storage.

Signed-off-by: Sergey Shilov <[email protected]>

* Tempoprary use leveldb as a default storage for the ledger.

Signed-off-by: Sergey Shilov <[email protected]>

* Adopt getAllTxn() for working with rocksdb iterator.

Signed-off-by: Sergey Shilov <[email protected]>

* Fix db_path property for leveldb and rocksdb, fix test.

Signed-off-by: Sergey Shilov <[email protected]>

* Add build procedure for python-rocksdb and setuptool, use librocksdb deb from sovrin.

Signed-off-by: Sergey Shilov <[email protected]>

* Add missed libs to docker file.

Signed-off-by: Sergey Shilov <[email protected]>

* Change rocksdb package.

Signed-off-by: Sergey Shilov <[email protected]>

* Change rocksdb package for 3d parties build.

Signed-off-by: Sergey Shilov <[email protected]>

* Implement get_equal_or_prev() functionality for KeyValueStorageRocksdbIntKeys.

Signed-off-by: Sergey Shilov <[email protected]>

* Add a helper for init of k/v storage with int keys.

Signed-off-by: Sergey Shilov <[email protected]>

* Add rocksdb tests for the equal-or-prev functionality.

Signed-off-by: Sergey Shilov <[email protected]>

* Fallback to leveldb as we do not want to migrate to rocksdb right now.

Signed-off-by: Sergey Shilov <[email protected]>
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