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

test(ef-tests, storage): do not sync_all static files in state tests #6914

Merged
merged 4 commits into from
Mar 7, 2024
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
1 change: 1 addition & 0 deletions crates/storage/nippy-jar/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,4 @@ tempfile.workspace = true

[features]
default = []
test-utils = []
30 changes: 28 additions & 2 deletions crates/storage/nippy-jar/src/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ impl<H: NippyJarHeader> NippyJarWriter<H> {

// First byte of the offset file is the size of one offset in bytes
offsets_file.write_all(&[OFFSET_SIZE_BYTES as u8])?;
offsets_file.sync_all()?;
joshieDo marked this conversation as resolved.
Show resolved Hide resolved
offsets_file.seek(SeekFrom::End(0))?;

Ok((data_file, offsets_file, is_created))
Expand Down Expand Up @@ -394,8 +393,36 @@ impl<H: NippyJarHeader> NippyJarWriter<H> {
Ok(())
}

#[cfg(feature = "test-utils")]
pub fn commit_without_sync_all(&mut self) -> Result<(), NippyJarError> {
self.data_file.flush()?;

self.commit_offsets_without_sync_all()?;

// Flushes `max_row_size` and total `rows` to disk.
self.jar.freeze_config()?;

Ok(())
}

/// Flushes offsets to disk.
pub(crate) fn commit_offsets(&mut self) -> Result<(), NippyJarError> {
self.commit_offsets_inner()?;
self.offsets_file.get_ref().sync_all()?;

Ok(())
}

#[cfg(feature = "test-utils")]
fn commit_offsets_without_sync_all(&mut self) -> Result<(), NippyJarError> {
self.commit_offsets_inner()
}

/// Flushes offsets to disk.
///
/// CAUTION: Does not call `sync_all` on the offsets file and requires a manual call to
/// `self.offsets_file.get_ref().sync_all()`.
fn commit_offsets_inner(&mut self) -> Result<(), NippyJarError> {
// The last offset on disk can be the first offset of `self.offsets` given how
// `append_column()` works alongside commit. So we need to skip it.
let mut last_offset_ondisk = None;
Expand All @@ -420,7 +447,6 @@ impl<H: NippyJarHeader> NippyJarWriter<H> {
self.offsets_file.write_all(&offset.to_le_bytes())?;
}
self.offsets_file.flush()?;
self.offsets_file.get_ref().sync_all()?;

Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/provider/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ assert_matches.workspace = true
rand.workspace = true

[features]
test-utils = ["alloy-rlp", "reth-db/test-utils"]
test-utils = ["alloy-rlp", "reth-db/test-utils", "reth-nippy-jar/test-utils"]
optimism = ["reth-primitives/optimism", "reth-interfaces/optimism"]
31 changes: 31 additions & 0 deletions crates/storage/provider/src/providers/static_file/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,37 @@ impl StaticFileProviderRW {
Ok(())
}

/// Commits configuration changes to disk and updates the reader index with the new changes.
///
/// CAUTION: does not call `sync_all` on the files.
#[cfg(feature = "test-utils")]
pub fn commit_without_sync_all(&mut self) -> ProviderResult<()> {
let start = Instant::now();

// Commits offsets and new user_header to disk
self.writer.commit_without_sync_all()?;

if let Some(metrics) = &self.metrics {
metrics.record_segment_operation(
self.writer.user_header().segment(),
StaticFileProviderOperation::CommitWriter,
Some(start.elapsed()),
);
}

debug!(
target: "provider::static_file",
segment = ?self.writer.user_header().segment(),
path = ?self.data_path,
duration = ?start.elapsed(),
"Commit"
);

self.update_index()?;

Ok(())
}

/// Updates the `self.reader` internal index.
fn update_index(&self) -> ProviderResult<()> {
// We find the maximum block of the segment by checking this writer's last block.
Expand Down
2 changes: 1 addition & 1 deletion testing/ef-tests/src/cases/blockchain_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ impl Case for BlockchainTestCase {
.static_file_provider()
.latest_writer(StaticFileSegment::Headers)
.unwrap()
.commit()
.commit_without_sync_all()
.unwrap();

// Execute the execution stage using the EVM processor factory for the test case
Expand Down
Loading