From 49fd38d4b1a7ad2bd23cd5f9d50969cb0e10f686 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Thu, 13 Jul 2023 14:43:42 +0200 Subject: [PATCH 1/6] refactor: remove duplicated code --- apps/src/lib/node/ledger/storage/rocksdb.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 9b6077f50a..f1ab743ce3 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -176,14 +176,6 @@ impl RocksDB { .ok_or(Error::DBError("No {cf_name} column family".to_string())) } - fn flush(&self, wait: bool) -> Result<()> { - let mut flush_opts = FlushOptions::default(); - flush_opts.set_wait(wait); - self.0 - .flush_opt(&flush_opts) - .map_err(|e| Error::DBError(e.into_string())) - } - /// Persist the diff of an account subspace key-val under the height where /// it was changed. fn write_subspace_diff( From c4fcad62db368fd9ceb5b05850848fb2ef1ec71c Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Thu, 13 Jul 2023 14:44:52 +0200 Subject: [PATCH 2/6] refactor: use immutable reference --- apps/src/lib/node/ledger/storage/rocksdb.rs | 2 +- core/src/ledger/storage/mockdb.rs | 2 +- core/src/ledger/storage/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index f1ab743ce3..681f6db4ed 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -504,7 +504,7 @@ impl DB for RocksDB { .map_err(|e| Error::DBError(e.into_string())) } - fn read_last_block(&mut self) -> Result> { + fn read_last_block(&self) -> Result> { // Block height let state_cf = self.get_column_family(STATE_CF)?; let height: BlockHeight = match self diff --git a/core/src/ledger/storage/mockdb.rs b/core/src/ledger/storage/mockdb.rs index 24ffd0e59b..5056d64cac 100644 --- a/core/src/ledger/storage/mockdb.rs +++ b/core/src/ledger/storage/mockdb.rs @@ -53,7 +53,7 @@ impl DB for MockDB { Ok(()) } - fn read_last_block(&mut self) -> Result> { + fn read_last_block(&self) -> Result> { // Block height let height: BlockHeight = match self.0.borrow().get("height") { Some(bytes) => types::decode(bytes).map_err(Error::CodingError)?, diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 5aa3d0cf80..73a70944ff 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -256,7 +256,7 @@ pub trait DB: std::fmt::Debug { fn flush(&self, wait: bool) -> Result<()>; /// Read the last committed block's metadata - fn read_last_block(&mut self) -> Result>; + fn read_last_block(&self) -> Result>; /// Write block's metadata. Merkle tree sub-stores are committed only when /// `is_full_commit` is `true` (typically on a beginning of a new epoch). From 32a10a42cd3a9c79aa0d993146100f9ee6d7ce39 Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Thu, 13 Jul 2023 14:46:48 +0200 Subject: [PATCH 3/6] refactor: use immutable reference --- apps/src/lib/node/ledger/storage/rocksdb.rs | 10 +++++----- core/src/ledger/storage/mockdb.rs | 2 +- core/src/ledger/storage/mod.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 681f6db4ed..7ada28c9ad 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -723,7 +723,7 @@ impl DB for RocksDB { } fn write_block( - &mut self, + &self, state: BlockStateWrite, batch: &mut Self::WriteBatch, is_full_commit: bool, @@ -1535,7 +1535,7 @@ mod test { ) .unwrap(); - write_block(&mut db, &mut batch, BlockHeight::default()).unwrap(); + write_block(&db, &mut batch, BlockHeight::default()).unwrap(); db.exec_batch(batch.0).unwrap(); let _state = db @@ -1726,7 +1726,7 @@ mod test { ) .unwrap(); - write_block(&mut db, &mut batch, height_0).unwrap(); + write_block(&db, &mut batch, height_0).unwrap(); db.exec_batch(batch.0).unwrap(); // Write second block @@ -1746,7 +1746,7 @@ mod test { db.batch_delete_subspace_val(&mut batch, height_1, &delete_key) .unwrap(); - write_block(&mut db, &mut batch, height_1).unwrap(); + write_block(&db, &mut batch, height_1).unwrap(); db.exec_batch(batch.0).unwrap(); // Check that the values are as expected from second block @@ -1771,7 +1771,7 @@ mod test { /// A test helper to write a block fn write_block( - db: &mut RocksDB, + db: &RocksDB, batch: &mut RocksDBWriteBatch, height: BlockHeight, ) -> Result<()> { diff --git a/core/src/ledger/storage/mockdb.rs b/core/src/ledger/storage/mockdb.rs index 5056d64cac..f5cfef09aa 100644 --- a/core/src/ledger/storage/mockdb.rs +++ b/core/src/ledger/storage/mockdb.rs @@ -212,7 +212,7 @@ impl DB for MockDB { } fn write_block( - &mut self, + &self, state: BlockStateWrite, _batch: &mut Self::WriteBatch, _is_full_commit: bool, diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 73a70944ff..5480eec51d 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -261,7 +261,7 @@ pub trait DB: std::fmt::Debug { /// Write block's metadata. Merkle tree sub-stores are committed only when /// `is_full_commit` is `true` (typically on a beginning of a new epoch). fn write_block( - &mut self, + &self, state: BlockStateWrite, batch: &mut Self::WriteBatch, is_full_commit: bool, From 20a110f2fab761868c08b1e21ca074a7da59c0ac Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Fri, 14 Jul 2023 12:47:30 +0200 Subject: [PATCH 4/6] refactor: rename method --- apps/src/lib/node/ledger/storage/rocksdb.rs | 4 ++-- core/src/ledger/storage/mockdb.rs | 2 +- core/src/ledger/storage/mod.rs | 5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 7ada28c9ad..6929603c77 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -722,7 +722,7 @@ impl DB for RocksDB { } } - fn write_block( + fn add_block_to_batch( &self, state: BlockStateWrite, batch: &mut Self::WriteBatch, @@ -1806,6 +1806,6 @@ mod test { eth_events_queue: ð_events_queue, }; - db.write_block(block, batch, true) + db.add_block_to_batch(block, batch, true) } } diff --git a/core/src/ledger/storage/mockdb.rs b/core/src/ledger/storage/mockdb.rs index f5cfef09aa..971584e742 100644 --- a/core/src/ledger/storage/mockdb.rs +++ b/core/src/ledger/storage/mockdb.rs @@ -211,7 +211,7 @@ impl DB for MockDB { } } - fn write_block( + fn add_block_to_batch( &self, state: BlockStateWrite, _batch: &mut Self::WriteBatch, diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index 5480eec51d..6509c02d34 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -260,7 +260,7 @@ pub trait DB: std::fmt::Debug { /// Write block's metadata. Merkle tree sub-stores are committed only when /// `is_full_commit` is `true` (typically on a beginning of a new epoch). - fn write_block( + fn add_block_to_batch( &self, state: BlockStateWrite, batch: &mut Self::WriteBatch, @@ -532,7 +532,8 @@ where ethereum_height: self.ethereum_height.as_ref(), eth_events_queue: &self.eth_events_queue, }; - self.db.write_block(state, &mut batch, is_full_commit)?; + self.db + .add_block_to_batch(state, &mut batch, is_full_commit)?; let header = self .header .take() From 5483cdc83e0e50b4f7c1aa877c6007e8bdaae51b Mon Sep 17 00:00:00 2001 From: Aleksandr Karbyshev Date: Fri, 14 Jul 2023 12:52:23 +0200 Subject: [PATCH 5/6] refactor: rename function --- apps/src/lib/node/ledger/storage/rocksdb.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/node/ledger/storage/rocksdb.rs b/apps/src/lib/node/ledger/storage/rocksdb.rs index 6929603c77..f679ca9232 100644 --- a/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -1535,7 +1535,7 @@ mod test { ) .unwrap(); - write_block(&db, &mut batch, BlockHeight::default()).unwrap(); + add_block_to_batch(&db, &mut batch, BlockHeight::default()).unwrap(); db.exec_batch(batch.0).unwrap(); let _state = db @@ -1726,7 +1726,7 @@ mod test { ) .unwrap(); - write_block(&db, &mut batch, height_0).unwrap(); + add_block_to_batch(&db, &mut batch, height_0).unwrap(); db.exec_batch(batch.0).unwrap(); // Write second block @@ -1746,7 +1746,7 @@ mod test { db.batch_delete_subspace_val(&mut batch, height_1, &delete_key) .unwrap(); - write_block(&db, &mut batch, height_1).unwrap(); + add_block_to_batch(&db, &mut batch, height_1).unwrap(); db.exec_batch(batch.0).unwrap(); // Check that the values are as expected from second block @@ -1770,7 +1770,7 @@ mod test { } /// A test helper to write a block - fn write_block( + fn add_block_to_batch( db: &RocksDB, batch: &mut RocksDBWriteBatch, height: BlockHeight, From 37bbbdcf30cfa2e4af163bda5c8ea98f72ba7f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Zemanovi=C4=8D?= Date: Fri, 14 Jul 2023 12:44:20 +0100 Subject: [PATCH 6/6] changelog: add #1717 --- .changelog/unreleased/improvements/1717-storage-refactor.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1717-storage-refactor.md diff --git a/.changelog/unreleased/improvements/1717-storage-refactor.md b/.changelog/unreleased/improvements/1717-storage-refactor.md new file mode 100644 index 0000000000..ae44ad180a --- /dev/null +++ b/.changelog/unreleased/improvements/1717-storage-refactor.md @@ -0,0 +1,2 @@ +- Refactored storage code to only use an immutable reference when reading and + writing to a batch. ([\#1717](https://github.com/anoma/namada/pull/1717)) \ No newline at end of file