-
Notifications
You must be signed in to change notification settings - Fork 691
Base implementation of RocksDB support #1416
Changes from 3 commits
3c114a4
f66ba87
0b1a67b
03bfb48
3279e60
6f5b702
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| version: 2.0 | ||
| version: 2.1 | ||
|
|
||
| # heavily inspired by: | ||
| # https://raw.githubusercontent.com/pinax/pinax-wiki/6bd2a99ab6f702e300d708532a6d1d9aa638b9f8/.circleci/config.yml | ||
|
|
@@ -23,7 +23,17 @@ common: &common | |
| when: on_fail | ||
| - restore_cache: | ||
| keys: | ||
| - cache-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| - cache-v1-python-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| - restore_cache: | ||
| keys: | ||
| - cache-v1-rocksdb-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum ".circleci/install_rocksdb.sh" }} | ||
| - run: | ||
| name: install rocksdb | ||
| command: sudo sh ./.circleci/install_rocksdb.sh | ||
| - save_cache: | ||
| paths: | ||
| - ~/rocksdb/ | ||
| key: cache-v1-rocksdb-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum ".circleci/install_rocksdb.sh" }} | ||
| - run: | ||
| name: install dependencies | ||
| command: pip install --user tox | ||
|
|
@@ -37,15 +47,25 @@ common: &common | |
| - ~/.cache/pip | ||
| - ~/.local | ||
| - ./eggs | ||
| key: cache-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| key: cache-v1-python-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
|
|
||
| geth_steps: &geth_steps | ||
| working_directory: ~/repo | ||
| steps: | ||
| - checkout | ||
| - restore_cache: | ||
| keys: | ||
| - cache-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| - cache-v1-python-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| - restore_cache: | ||
| keys: | ||
| - cache-v2-rocksdb-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum ".circleci/install_rocksdb.sh" }} | ||
| - run: | ||
| name: install rocksdb | ||
| command: sudo sh ./.circleci/install_rocksdb.sh | ||
| - save_cache: | ||
| paths: | ||
| - ~/rocksdb/ | ||
| key: cache-v2-rocksdb-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum ".circleci/install_rocksdb.sh" }} | ||
| - run: | ||
| name: install dependencies | ||
| command: pip install --user tox | ||
|
|
@@ -70,7 +90,7 @@ geth_steps: &geth_steps | |
| geth version | ||
| - run: | ||
| name: run tox | ||
| command: ~/.local/bin/tox -r | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was helping us quickly catch when a live change in a dependency was causing problems with py-evm. Was the change for performance? How much shorter was the test?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another thing that didn't get reverted. will remove. |
||
| command: ~/.local/bin/tox | ||
| - save_cache: | ||
| paths: | ||
| - .tox | ||
|
|
@@ -79,7 +99,7 @@ geth_steps: &geth_steps | |
| - ./eggs | ||
| - ~/.ethash | ||
| - ~/.py-geth | ||
| key: cache-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
| key: cache-v1-python-{{ arch }}-{{ .Environment.CIRCLE_JOB }}-{{ checksum "setup.py" }}-{{ checksum "tox.ini" }} | ||
|
|
||
| jobs: | ||
| py35-lint: | ||
|
|
@@ -266,7 +286,6 @@ jobs: | |
| environment: | ||
| TOXENV: py36-trinity-integration | ||
| py36-trinity-lightchain_integration: | ||
| <<: *common | ||
| <<: *geth_steps | ||
| docker: | ||
| - image: circleci/python:3.6 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -e | ||
| set -u | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find the long-form nicer for readability, like: set -o errexit
set -o nounset
set -o pipefail
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL there is a longform. |
||
|
|
||
| sudo apt-get install -y liblz4-dev libsnappy-dev libgflags-dev zlib1g-dev libbz2-dev libzstd-dev | ||
|
|
||
|
|
||
| if [ ! -d "/home/circleci/rocksdb" ]; then | ||
| git clone https://github.com/facebook/rocksdb /home/circleci/rocksdb | ||
| fi | ||
| if [ ! -f "/home/circleci/rocksdb/librocksdb.so.5.8.8" ]; then | ||
| cd /home/circleci/rocksdb/ && git checkout v5.8.8 && sudo make install-shared INSTALL_PATH=/usr | ||
| fi | ||
| if [ ! -f "/usr/lib/librocksdb.so.5.8" ]; then | ||
| ln -fs /home/circleci/rocksdb/librocksdb.so.5.8.8 /usr/lib/librocksdb.so.5.8 | ||
| fi | ||
| if [ ! -f "/usr/lib/librocksdb.so.5" ]; then | ||
| ln -fs /home/circleci/rocksdb/librocksdb.so.5.8.8 /usr/lib/librocksdb.so.5 | ||
| fi | ||
| if [ ! -f "/usr/lib/librocksdb.so" ]; then | ||
| ln -fs /home/circleci/rocksdb/librocksdb.so.5.8.8 /usr/lib/librocksdb.so | ||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,12 @@ we need to install the ``python3-pip`` package through the following command. | |
|
|
||
| apt-get install python3-pip | ||
|
|
||
| Trinity also requires RocksDB which can be installed with the following command: | ||
|
|
||
| .. code:: sh | ||
|
|
||
| apt-get install liblz4-dev lib-rocksdb5.8 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this doc get stale? I suppose there's a reason you had to switch circle to use: sudo apt-get install -y liblz4-dev libsnappy-dev libgflags-dev zlib1g-dev libbz2-dev libzstd-dev
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not stale. Requirements for rocksdb installation, will double check but I believe these are directly from their readme. On a related note, I think our installation story is about to get ungood and we should look into debian packages and/or brew. We're gonna have to do it eventually... |
||
|
|
||
| .. note:: | ||
| .. include:: /fragments/virtualenv_explainer.rst | ||
|
|
||
|
|
@@ -42,7 +48,7 @@ First, install LevelDB and the latest Python 3 with brew: | |
|
|
||
| .. code:: sh | ||
|
|
||
| brew install python3 leveldb | ||
| brew install python3 leveldb rocksdb | ||
|
|
||
| .. note:: | ||
| .. include:: /fragments/virtualenv_explainer.rst | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,10 +27,9 @@ | |
| class LevelDB(BaseAtomicDB): | ||
| logger = logging.getLogger("eth.db.backends.LevelDB") | ||
|
|
||
| # Creates db as a class variable to avoid level db lock error | ||
| def __init__(self, db_path: Path = None) -> None: | ||
| if not db_path: | ||
| raise TypeError("Please specifiy a valid path for your database.") | ||
| raise TypeError("The LevelDB backend requires a database path") | ||
| try: | ||
| with catch_and_ignore_import_warning(): | ||
| import plyvel # noqa: F811 | ||
|
|
@@ -54,6 +53,9 @@ def _exists(self, key: bytes) -> bool: | |
| return self.db.get(key) is not None | ||
|
|
||
| def __delitem__(self, key: bytes) -> None: | ||
| v = self.db.get(key) | ||
| if v is None: | ||
| raise KeyError(key) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without this the leveldb backend isn't fully compliant with the |
||
| self.db.delete(key) | ||
|
|
||
| @contextmanager | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| from contextlib import contextmanager | ||
| import logging | ||
| from pathlib import Path | ||
| from typing import ( | ||
| Generator, | ||
| TYPE_CHECKING, | ||
| ) | ||
|
|
||
| from eth_utils import ValidationError | ||
|
|
||
| from eth.db.diff import ( | ||
| DBDiffTracker, | ||
| DiffMissingError, | ||
| ) | ||
| from .base import ( | ||
| BaseAtomicDB, | ||
| BaseDB, | ||
| ) | ||
|
|
||
| if TYPE_CHECKING: | ||
| import rocksdb # noqa: F401 | ||
|
|
||
|
|
||
| class RocksDB(BaseAtomicDB): | ||
| logger = logging.getLogger("eth.db.backends.RocksDB") | ||
|
|
||
| def __init__(self, | ||
| db_path: Path = None, | ||
| opts: 'rocksdb.Options' = None, | ||
| read_only: bool=False) -> None: | ||
| if not db_path: | ||
| raise TypeError("The RocksDB backend requires a database path") | ||
| try: | ||
| import rocksdb # noqa: F811 | ||
| except ImportError: | ||
| raise ImportError( | ||
| "RocksDB requires the python-rocksdb library which is not " | ||
| "available for import." | ||
| ) | ||
|
|
||
| if opts is None: | ||
| opts = rocksdb.Options(create_if_missing=True) | ||
| self.db_path = db_path | ||
| self.db = rocksdb.DB(str(db_path), opts, read_only=read_only) | ||
|
|
||
| def __getitem__(self, key: bytes) -> bytes: | ||
| v = self.db.get(key) | ||
| if v is None: | ||
| raise KeyError(key) | ||
| return v | ||
|
|
||
| def __setitem__(self, key: bytes, value: bytes) -> None: | ||
| self.db.put(key, value) | ||
|
|
||
| def _exists(self, key: bytes) -> bool: | ||
| return self.db.get(key) is not None | ||
|
|
||
| def __delitem__(self, key: bytes) -> None: | ||
| exists, _ = self.db.key_may_exist(key) | ||
| if not exists: | ||
| raise KeyError(key) | ||
| self.db.delete(key) | ||
|
|
||
| @contextmanager | ||
| def atomic_batch(self) -> Generator['RocksDBWriteBatch', None, None]: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. likely copy/paste from |
||
| import rocksdb # noqa: F811 | ||
| batch = rocksdb.WriteBatch() | ||
|
|
||
| readable_batch = RocksDBWriteBatch(self, batch) | ||
|
|
||
| try: | ||
| yield readable_batch | ||
| finally: | ||
| readable_batch.decommission() | ||
|
|
||
| self.db.write(batch) | ||
|
|
||
|
|
||
| class RocksDBWriteBatch(BaseDB): | ||
| """ | ||
| A native rocksdb write batch does not permit reads on the in-progress data. | ||
| This class fills that gap, by tracking the in-progress diff, and adding | ||
| a read interface. | ||
| """ | ||
| logger = logging.getLogger("eth.db.backends.RocksDBWriteBatch") | ||
|
|
||
| def __init__(self, original_read_db: BaseDB, write_batch: 'rocksdb.WriteBatch') -> None: | ||
| self._original_read_db = original_read_db | ||
| self._write_batch = write_batch | ||
| # keep track of the temporary changes made | ||
| self._track_diff = DBDiffTracker() | ||
|
|
||
| def __getitem__(self, key: bytes) -> bytes: | ||
| if self._track_diff is None: | ||
| raise ValidationError("Cannot get data from a write batch, out of context") | ||
|
|
||
| try: | ||
| changed_value = self._track_diff[key] | ||
| except DiffMissingError as missing: | ||
| if missing.is_deleted: | ||
| raise KeyError(key) | ||
| else: | ||
| return self._original_read_db[key] | ||
| else: | ||
| return changed_value | ||
|
|
||
| def __setitem__(self, key: bytes, value: bytes) -> None: | ||
| if self._track_diff is None: | ||
| raise ValidationError("Cannot set data from a write batch, out of context") | ||
|
|
||
| self._write_batch.put(key, value) | ||
| self._track_diff[key] = value | ||
|
|
||
| def _exists(self, key: bytes) -> bool: | ||
| if self._track_diff is None: | ||
| raise ValidationError("Cannot test data existance from a write batch, out of context") | ||
|
|
||
| try: | ||
| self._track_diff[key] | ||
| except DiffMissingError as missing: | ||
| if missing.is_deleted: | ||
| return False | ||
| else: | ||
| return key in self._original_read_db | ||
| else: | ||
| return True | ||
|
|
||
| def __delitem__(self, key: bytes) -> None: | ||
| if self._track_diff is None: | ||
| raise ValidationError("Cannot delete data from a write batch, out of context") | ||
|
|
||
| self._write_batch.delete(key) | ||
| del self._track_diff[key] | ||
|
|
||
| def decommission(self) -> None: | ||
| """ | ||
| Prevent any further actions to be taken on this write batch, called after leaving context | ||
| """ | ||
| self._track_diff = None | ||
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.
Just curious, what required this upgrade?
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.
It's superfluous. I should remove it.