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

feat(estimator): compact RocksDB in testbed setup #7502

Merged
merged 7 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/store/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ pub trait Database: Sync + Send {
/// This is a no-op for in-memory databases.
fn flush(&self) -> io::Result<()>;

/// Compact database representation.
///
/// If the database supports it a form of compaction, calling this function
/// is blocking until compaction finishes. Otherwise, this is a no-op.
Comment on lines +140 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Compact database representation.
///
/// If the database supports it a form of compaction, calling this function
/// is blocking until compaction finishes. Otherwise, this is a no-op.
/// Compacts database representation.
///
/// If the database supports compaction, calling this function blocks
/// until compaction finishes. Otherwise, this is a no-op.

fn compact(&self) -> io::Result<()>;

/// Returns statistics about the database if available.
fn get_store_statistics(&self) -> Option<StoreStatistics>;
}
Expand Down
8 changes: 8 additions & 0 deletions core/store/src/db/rocksdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,14 @@ impl Database for RocksDB {
self.db.write(batch).map_err(into_other)
}

fn compact(&self) -> io::Result<()> {
let none = Option::<&[u8]>::None;
Copy link
Contributor

Choose a reason for hiding this comment

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

And that's why I have a distace for AsRef-polymorphism for nicer APIs :)

for col in DBCol::iter() {
self.db.compact_range_cf(self.cf_handle(col), none, none);
}
Ok(())
}

fn flush(&self) -> io::Result<()> {
self.db.flush().map_err(into_other)
}
Expand Down
4 changes: 4 additions & 0 deletions core/store/src/db/testdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ impl Database for TestDB {
Ok(())
}

fn compact(&self) -> io::Result<()> {
Ok(())
}

fn get_store_statistics(&self) -> Option<StoreStatistics> {
None
}
Expand Down
5 changes: 5 additions & 0 deletions core/store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ impl Store {
self.storage.flush()
}

/// Blocking compaction request if supported by storage.
pub fn compact(&self) -> io::Result<()> {
self.storage.compact()
}

pub fn get_store_statistics(&self) -> Option<StoreStatistics> {
self.storage.get_store_statistics()
}
Expand Down
11 changes: 11 additions & 0 deletions runtime/runtime-params-estimator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,17 @@
//! costs and don't want to just run everything in order (as that would be to
//! slow), we have a very simple manual caching infrastructure in place.
//!
//! To run estimations on a non-empty DB with standardised content, we first
//! dump all records to a `StateDump` written to a file. Then for each
//! iteration of a an estimation, we first load the records from this dump into
//! a fresh database. Afterwards, it is crucial to run compaction on RocksDB
//! before starting measurements. Otherwise, the SST file layout can be very
//! inefficient, as there was no time to restructure them. We assume that in
//! production, the inflow of new data is not as bulky and therefore it should
//! always be reasonably compacted. Also, without forcing it before
//! measurements start, compaction may start during the measurement and makes
//! the results unstable.
//!
//! Notes on code architecture:
//!
//! To keep estimations comprehensible, each estimation has a simple function
Expand Down
2 changes: 2 additions & 0 deletions runtime/runtime-params-estimator/src/testbed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ impl RuntimeTestbed {
pub fn from_state_dump(dump_dir: &Path) -> Self {
let workdir = tempfile::Builder::new().prefix("runtime_testbed").tempdir().unwrap();
let StateDump { store, roots } = StateDump::from_dir(dump_dir, workdir.path());
// Ensure decent RocksDB SST file layout.
store.compact().expect("compaction failed");
let tries = ShardTries::test(store, 1);

assert!(roots.len() <= 1, "Parameter estimation works with one shard only.");
Expand Down