From 7a4f2dbe043153eee7e85b5699f1d614c05373d1 Mon Sep 17 00:00:00 2001 From: Gabriel Levcovitz Date: Wed, 28 Aug 2024 18:35:43 -0300 Subject: [PATCH] feat: enable cache by default --- hathor/builder/cli_builder.py | 23 +++++++++++++++++++---- hathor/cli/run_node.py | 4 +++- hathor/cli/run_node_args.py | 1 + tests/others/test_cli_builder.py | 20 +++++++++++--------- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/hathor/builder/cli_builder.py b/hathor/builder/cli_builder.py index 3f10d7454..4548c5077 100644 --- a/hathor/builder/cli_builder.py +++ b/hathor/builder/cli_builder.py @@ -50,6 +50,8 @@ logger = get_logger() +DEFAULT_CACHE_SIZE: int = 100000 + class SyncChoice(Enum): V1_DEFAULT = auto() # v1 enabled, v2 disabled but can be enabled in runtime @@ -150,7 +152,7 @@ def create_manager(self, reactor: Reactor) -> HathorManager: indexes = RocksDBIndexesManager(self.rocksdb_storage) kwargs: dict[str, Any] = {} - if not self._args.cache: + if self._args.disable_cache: # We should only pass indexes if cache is disabled. Otherwise, # only TransactionCacheStorage should have indexes. kwargs['indexes'] = indexes @@ -161,14 +163,27 @@ def create_manager(self, reactor: Reactor) -> HathorManager: feature_storage = FeatureActivationStorage(settings=settings, rocksdb_storage=self.rocksdb_storage) self.log.info('with storage', storage_class=type(tx_storage).__name__, path=self._args.data) + if self._args.cache: - self.check_or_raise(not self._args.memory_storage, '--cache should not be used with --memory-storage') - tx_storage = TransactionCacheStorage(tx_storage, reactor, indexes=indexes, settings=settings) + self.log.warn('--cache is now the default and will be removed') + + if self._args.disable_cache: + self.check_or_raise(self._args.cache_size is None, 'cannot use --disable-cache with --cache-size') + self.check_or_raise(self._args.cache_interval is None, 'cannot use --disable-cache with --cache-interval') + + if self._args.memory_storage: if self._args.cache_size: - tx_storage.capacity = self._args.cache_size + self.log.warn('using --cache-size with --memory-storage has no effect') + if self._args.cache_interval: + self.log.warn('using --cache-interval with --memory-storage has no effect') + + if not self._args.disable_cache and not self._args.memory_storage: + tx_storage = TransactionCacheStorage(tx_storage, reactor, indexes=indexes, settings=settings) + tx_storage.capacity = self._args.cache_size if self._args.cache_size is not None else DEFAULT_CACHE_SIZE if self._args.cache_interval: tx_storage.interval = self._args.cache_interval self.log.info('with cache', capacity=tx_storage.capacity, interval=tx_storage.interval) + self.tx_storage = tx_storage self.log.info('with indexes', indexes_class=type(tx_storage.indexes).__name__) diff --git a/hathor/cli/run_node.py b/hathor/cli/run_node.py index a7d7be0d6..88489fa8a 100644 --- a/hathor/cli/run_node.py +++ b/hathor/cli/run_node.py @@ -116,7 +116,9 @@ def create_parser(cls) -> ArgumentParser: parser.add_argument('--prometheus', action='store_true', help='Send metric data to Prometheus') parser.add_argument('--prometheus-prefix', default='', help='A prefix that will be added in all Prometheus metrics') - parser.add_argument('--cache', action='store_true', help='Use cache for tx storage') + cache_args = parser.add_mutually_exclusive_group() + cache_args.add_argument('--cache', action='store_true', help=SUPPRESS) # moved to --disable-cache + cache_args.add_argument('--disable-cache', action='store_true', help='Disable cache for tx storage') parser.add_argument('--cache-size', type=int, help='Number of txs to keep on cache') parser.add_argument('--cache-interval', type=int, help='Cache flush interval') parser.add_argument('--recursion-limit', type=int, help='Set python recursion limit') diff --git a/hathor/cli/run_node_args.py b/hathor/cli/run_node_args.py index 36b137e3f..f493a7d33 100644 --- a/hathor/cli/run_node_args.py +++ b/hathor/cli/run_node_args.py @@ -52,6 +52,7 @@ class RunNodeArgs(BaseModel, extra=Extra.allow): prometheus: bool prometheus_prefix: str cache: bool + disable_cache: bool cache_size: Optional[int] cache_interval: Optional[int] recursion_limit: Optional[int] diff --git a/tests/others/test_cli_builder.py b/tests/others/test_cli_builder.py index 96a4aaeca..a83f00899 100644 --- a/tests/others/test_cli_builder.py +++ b/tests/others/test_cli_builder.py @@ -53,7 +53,8 @@ def test_empty(self): def test_all_default(self): data_dir = self.mkdtemp() manager = self._build(['--data', data_dir]) - self.assertIsInstance(manager.tx_storage, TransactionRocksDBStorage) + self.assertIsInstance(manager.tx_storage, TransactionCacheStorage) + self.assertIsInstance(manager.tx_storage.store, TransactionRocksDBStorage) self.assertIsInstance(manager.tx_storage.indexes, RocksDBIndexesManager) self.assertIsNone(manager.wallet) self.assertEqual('unittests', manager.network) @@ -64,33 +65,34 @@ def test_all_default(self): self.assertFalse(manager._enable_event_queue) @pytest.mark.skipif(not HAS_ROCKSDB, reason='requires python-rocksdb') - def test_cache_storage(self): + def test_disable_cache_storage(self): data_dir = self.mkdtemp() - manager = self._build(['--cache', '--data', data_dir]) - self.assertIsInstance(manager.tx_storage, TransactionCacheStorage) - self.assertIsInstance(manager.tx_storage.store, TransactionRocksDBStorage) + manager = self._build(['--disable-cache', '--data', data_dir]) + self.assertIsInstance(manager.tx_storage, TransactionRocksDBStorage) self.assertIsInstance(manager.tx_storage.indexes, RocksDBIndexesManager) - self.assertIsNone(manager.tx_storage.store.indexes) @pytest.mark.skipif(not HAS_ROCKSDB, reason='requires python-rocksdb') def test_default_storage_memory_indexes(self): data_dir = self.mkdtemp() manager = self._build(['--memory-indexes', '--data', data_dir]) - self.assertIsInstance(manager.tx_storage, TransactionRocksDBStorage) + self.assertIsInstance(manager.tx_storage, TransactionCacheStorage) + self.assertIsInstance(manager.tx_storage.store, TransactionRocksDBStorage) self.assertIsInstance(manager.tx_storage.indexes, MemoryIndexesManager) @pytest.mark.skipif(not HAS_ROCKSDB, reason='requires python-rocksdb') def test_default_storage_with_rocksdb_indexes(self): data_dir = self.mkdtemp() manager = self._build(['--x-rocksdb-indexes', '--data', data_dir]) - self.assertIsInstance(manager.tx_storage, TransactionRocksDBStorage) + self.assertIsInstance(manager.tx_storage, TransactionCacheStorage) + self.assertIsInstance(manager.tx_storage.store, TransactionRocksDBStorage) self.assertIsInstance(manager.tx_storage.indexes, RocksDBIndexesManager) @pytest.mark.skipif(not HAS_ROCKSDB, reason='requires python-rocksdb') def test_rocksdb_storage(self): data_dir = self.mkdtemp() manager = self._build(['--rocksdb-storage', '--data', data_dir]) - self.assertIsInstance(manager.tx_storage, TransactionRocksDBStorage) + self.assertIsInstance(manager.tx_storage, TransactionCacheStorage) + self.assertIsInstance(manager.tx_storage.store, TransactionRocksDBStorage) self.assertIsInstance(manager.tx_storage.indexes, RocksDBIndexesManager) def test_memory_storage(self):