From a4f6c4181aa7f975b1c3aad8d3fb30021388e014 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Wed, 1 Mar 2023 18:55:51 +0100 Subject: feat(tvix/store): add new_temporary for all Sled services This provides a service using /dev/shm, that's deleted once the reference is dropped. Refactor all tests to use these, which allows getting rid of most TempDir usage in the tests. The only place where we still use TempDir is in the importer tests, which work on a filesystem path. Change-Id: I08a950aa774bf9b46d9f5c92edf5efba36053242 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8193 Reviewed-by: raitobezarius Tested-by: BuildkiteCI --- tvix/store/src/blobreader.rs | 19 +++----- tvix/store/src/blobservice/sled.rs | 7 +++ tvix/store/src/chunkservice/sled.rs | 7 +++ tvix/store/src/directoryservice/sled.rs | 7 +++ tvix/store/src/pathinfoservice/sled.rs | 7 +++ tvix/store/src/proto/tests/grpc_blobservice.rs | 18 +++---- .../store/src/proto/tests/grpc_directoryservice.rs | 17 +++---- tvix/store/src/proto/tests/grpc_pathinfoservice.rs | 18 +++---- tvix/store/src/tests/import.rs | 56 +++++++++++----------- tvix/store/src/tests/nar_renderer.rs | 51 ++++++-------------- tvix/store/src/tests/utils.rs | 18 ++++--- 11 files changed, 107 insertions(+), 118 deletions(-) diff --git a/tvix/store/src/blobreader.rs b/tvix/store/src/blobreader.rs index b4e56d909b26..4da0f9593a58 100644 --- a/tvix/store/src/blobreader.rs +++ b/tvix/store/src/blobreader.rs @@ -141,13 +141,11 @@ mod tests { use std::io::Cursor; use std::io::Read; use std::io::Write; - use tempfile::TempDir; #[test] /// reading from a blobmeta with zero chunks should produce zero bytes. fn empty_blobmeta() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); let blobmeta = proto::BlobMeta { chunks: vec![], @@ -167,8 +165,7 @@ mod tests { #[test] /// trying to read something where the chunk doesn't exist should fail fn missing_chunk_fail() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); let blobmeta = proto::BlobMeta { chunks: vec![proto::blob_meta::ChunkMeta { @@ -191,8 +188,7 @@ mod tests { #[test] /// read something containing the single (empty) chunk fn empty_chunk() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); // insert a single chunk let dgst = chunk_service.put(vec![]).expect("must succeed"); @@ -222,8 +218,7 @@ mod tests { /// read something which contains a single chunk #[test] fn single_chunk() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); // insert a single chunk let dgst = chunk_service @@ -255,8 +250,7 @@ mod tests { /// read something referring to a chunk, but with wrong size #[test] fn wrong_size_fail() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); // insert chunks let dgst_1 = chunk_service @@ -286,8 +280,7 @@ mod tests { /// read something referring to multiple chunks #[test] fn multiple_chunks() -> anyhow::Result<()> { - let tmpdir = TempDir::new()?; - let chunk_service = gen_chunk_service(tmpdir.path()); + let chunk_service = gen_chunk_service(); // insert chunks let dgst_1 = chunk_service diff --git a/tvix/store/src/blobservice/sled.rs b/tvix/store/src/blobservice/sled.rs index 729a520ac2d6..1ae34ee5fb9c 100644 --- a/tvix/store/src/blobservice/sled.rs +++ b/tvix/store/src/blobservice/sled.rs @@ -20,6 +20,13 @@ impl SledBlobService { Ok(Self { db }) } + + pub fn new_temporary() -> Result { + let config = sled::Config::default().temporary(true); + let db = config.open()?; + + Ok(Self { db }) + } } impl BlobService for SledBlobService { diff --git a/tvix/store/src/chunkservice/sled.rs b/tvix/store/src/chunkservice/sled.rs index 06194ddc5943..5cb469b2663d 100644 --- a/tvix/store/src/chunkservice/sled.rs +++ b/tvix/store/src/chunkservice/sled.rs @@ -19,6 +19,13 @@ impl SledChunkService { Ok(Self { db }) } + + pub fn new_temporary() -> Result { + let config = sled::Config::default().temporary(true); + let db = config.open()?; + + Ok(Self { db }) + } } impl ChunkService for SledChunkService { diff --git a/tvix/store/src/directoryservice/sled.rs b/tvix/store/src/directoryservice/sled.rs index caa78615d30a..064f86ceaa6e 100644 --- a/tvix/store/src/directoryservice/sled.rs +++ b/tvix/store/src/directoryservice/sled.rs @@ -19,6 +19,13 @@ impl SledDirectoryService { Ok(Self { db }) } + + pub fn new_temporary() -> Result { + let config = sled::Config::default().temporary(true); + let db = config.open()?; + + Ok(Self { db }) + } } impl DirectoryService for SledDirectoryService { diff --git a/tvix/store/src/pathinfoservice/sled.rs b/tvix/store/src/pathinfoservice/sled.rs index dc3c95b51044..b629d869f0fe 100644 --- a/tvix/store/src/pathinfoservice/sled.rs +++ b/tvix/store/src/pathinfoservice/sled.rs @@ -21,6 +21,13 @@ impl SledPathInfoService { Ok(Self { db }) } + + pub fn new_temporary() -> Result { + let config = sled::Config::default().temporary(true); + let db = config.open()?; + + Ok(Self { db }) + } } impl PathInfoService for SledPathInfoService { diff --git a/tvix/store/src/proto/tests/grpc_blobservice.rs b/tvix/store/src/proto/tests/grpc_blobservice.rs index 4ea367cbdc86..88eb21e30241 100644 --- a/tvix/store/src/proto/tests/grpc_blobservice.rs +++ b/tvix/store/src/proto/tests/grpc_blobservice.rs @@ -5,24 +5,20 @@ use crate::proto::blob_service_server::BlobService as GRPCBlobService; use crate::proto::{BlobChunk, GRPCBlobServiceWrapper, ReadBlobRequest, StatBlobRequest}; use crate::tests::fixtures::{BLOB_A, BLOB_A_DIGEST, BLOB_B, BLOB_B_DIGEST}; use crate::tests::utils::{gen_blob_service, gen_chunk_service}; -use std::path::Path; -use tempfile::TempDir; -fn gen_grpc_blob_service( - p: &Path, -) -> GRPCBlobServiceWrapper< +fn gen_grpc_blob_service() -> GRPCBlobServiceWrapper< impl BlobService + Send + Sync + Clone + 'static, impl ChunkService + Send + Sync + Clone + 'static, > { - let blob_service = gen_blob_service(p); - let chunk_service = gen_chunk_service(p); + let blob_service = gen_blob_service(); + let chunk_service = gen_chunk_service(); GRPCBlobServiceWrapper::new(blob_service, chunk_service) } /// Trying to read a non-existent blob should return a not found error. #[tokio::test] async fn not_found_read() { - let service = gen_grpc_blob_service(TempDir::new().unwrap().path()); + let service = gen_grpc_blob_service(); let resp = service .read(tonic::Request::new(ReadBlobRequest { @@ -37,7 +33,7 @@ async fn not_found_read() { /// Trying to stat a non-existent blob should return a not found error. #[tokio::test] async fn not_found_stat() { - let service = gen_grpc_blob_service(TempDir::new().unwrap().path()); + let service = gen_grpc_blob_service(); let resp = service .stat(tonic::Request::new(StatBlobRequest { @@ -55,7 +51,7 @@ async fn not_found_stat() { /// won't get split into multiple chunks. #[tokio::test] async fn put_read_stat() { - let service = gen_grpc_blob_service(TempDir::new().unwrap().path()); + let service = gen_grpc_blob_service(); // Send blob A. let put_resp = service @@ -117,7 +113,7 @@ async fn put_read_stat() { /// `read()` method. #[tokio::test] async fn put_read_stat_large() { - let service = gen_grpc_blob_service(TempDir::new().unwrap().path()); + let service = gen_grpc_blob_service(); // split up BLOB_B into BlobChunks containing 1K bytes each. let blob_b_blobchunks: Vec = BLOB_B diff --git a/tvix/store/src/proto/tests/grpc_directoryservice.rs b/tvix/store/src/proto/tests/grpc_directoryservice.rs index 6d9851111f17..464c25f33d75 100644 --- a/tvix/store/src/proto/tests/grpc_directoryservice.rs +++ b/tvix/store/src/proto/tests/grpc_directoryservice.rs @@ -5,15 +5,12 @@ use crate::proto::{Directory, DirectoryNode, SymlinkNode}; use crate::proto::{GRPCDirectoryServiceWrapper, GetDirectoryRequest}; use crate::tests::fixtures::{DIRECTORY_A, DIRECTORY_B, DIRECTORY_C}; use crate::tests::utils::gen_directory_service; -use std::path::Path; -use tempfile::TempDir; use tokio_stream::StreamExt; use tonic::Status; fn gen_grpc_service( - p: &Path, ) -> GRPCDirectoryServiceWrapper { - let directory_service = gen_directory_service(p); + let directory_service = gen_directory_service(); GRPCDirectoryServiceWrapper::from(directory_service) } @@ -43,7 +40,7 @@ async fn get_directories( /// Trying to get a non-existent Directory should return a not found error. #[tokio::test] async fn not_found() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); let resp = service .get(tonic::Request::new(GetDirectoryRequest { @@ -69,7 +66,7 @@ async fn not_found() { /// Put a Directory into the store, get it back. #[tokio::test] async fn put_get() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); let streaming_request = tonic_mock::streaming_request(vec![DIRECTORY_A.clone()]); let put_resp = service @@ -98,7 +95,7 @@ async fn put_get() { /// Put multiple Directories into the store, and get them back #[tokio::test] async fn put_get_multiple() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); // sending "b" (which refers to "a") without sending "a" first should fail. let put_resp = service @@ -151,7 +148,7 @@ async fn put_get_multiple() { /// Put multiple Directories into the store, and omit duplicates. #[tokio::test] async fn put_get_dedup() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); // Send "A", then "C", which refers to "A" two times // Pretend we're a dumb client sending A twice. @@ -184,7 +181,7 @@ async fn put_get_dedup() { /// Trying to upload a Directory failing validation should fail. #[tokio::test] async fn put_reject_failed_validation() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); // construct a broken Directory message that fails validation let broken_directory = Directory { @@ -208,7 +205,7 @@ async fn put_reject_failed_validation() { /// Trying to upload a Directory with wrong size should fail. #[tokio::test] async fn put_reject_wrong_size() { - let service = gen_grpc_service(TempDir::new().unwrap().path()); + let service = gen_grpc_service(); // Construct a directory referring to DIRECTORY_A, but with wrong size. let broken_parent_directory = Directory { diff --git a/tvix/store/src/proto/tests/grpc_pathinfoservice.rs b/tvix/store/src/proto/tests/grpc_pathinfoservice.rs index ce74711cb555..f74c3fda0213 100644 --- a/tvix/store/src/proto/tests/grpc_pathinfoservice.rs +++ b/tvix/store/src/proto/tests/grpc_pathinfoservice.rs @@ -9,8 +9,6 @@ use crate::tests::fixtures::DUMMY_OUTPUT_HASH; use crate::tests::utils::{ gen_blob_service, gen_chunk_service, gen_directory_service, gen_pathinfo_service, }; -use std::path::Path; -use tempfile::TempDir; use tonic::Request; /// generates a GRPCPathInfoService out of blob, chunk, directory and pathinfo services. @@ -18,13 +16,13 @@ use tonic::Request; /// We only interact with it via the PathInfo GRPC interface. /// It uses the NonCachingNARCalculationService NARCalculationService to /// calculate NARs. -fn gen_grpc_service(p: &Path) -> impl GRPCPathInfoService { +fn gen_grpc_service() -> impl GRPCPathInfoService { GRPCPathInfoServiceWrapper::new( - gen_pathinfo_service(p), + gen_pathinfo_service(), NonCachingNARCalculationService::new( - gen_blob_service(p), - gen_chunk_service(p), - gen_directory_service(p), + gen_blob_service(), + gen_chunk_service(), + gen_directory_service(), ), ) } @@ -32,8 +30,7 @@ fn gen_grpc_service(p: &Path) -> impl GRPCPathInfoService { /// Trying to get a non-existent PathInfo should return a not found error. #[tokio::test] async fn not_found() { - let tmpdir = TempDir::new().unwrap(); - let service = gen_grpc_service(tmpdir.path()); + let service = gen_grpc_service(); let resp = service .get(Request::new(GetPathInfoRequest { @@ -48,8 +45,7 @@ async fn not_found() { /// Put a PathInfo into the store, get it back. #[tokio::test] async fn put_get() { - let tmpdir = TempDir::new().unwrap(); - let service = gen_grpc_service(tmpdir.path()); + let service = gen_grpc_service(); let path_info = PathInfo { node: Some(Node { diff --git a/tvix/store/src/tests/import.rs b/tvix/store/src/tests/import.rs index e8373b614e91..63ca1efa13f3 100644 --- a/tvix/store/src/tests/import.rs +++ b/tvix/store/src/tests/import.rs @@ -12,15 +12,18 @@ use tempfile::TempDir; fn symlink() { let tmpdir = TempDir::new().unwrap(); - let data_dir = tmpdir.path().join("data"); - std::fs::create_dir_all(&data_dir).unwrap(); - std::os::unix::fs::symlink("/nix/store/somewhereelse", data_dir.join("doesntmatter")).unwrap(); + std::fs::create_dir_all(&tmpdir).unwrap(); + std::os::unix::fs::symlink( + "/nix/store/somewhereelse", + tmpdir.path().join("doesntmatter"), + ) + .unwrap(); let root_node = import_path( - &mut gen_blob_service(tmpdir.path()), - &mut gen_chunk_service(tmpdir.path()), - &mut gen_directory_service(tmpdir.path()), - data_dir.join("doesntmatter"), + &mut gen_blob_service(), + &mut gen_chunk_service(), + &mut gen_directory_service(), + tmpdir.path().join("doesntmatter"), ) .expect("must succeed"); @@ -37,17 +40,15 @@ fn symlink() { fn single_file() { let tmpdir = TempDir::new().unwrap(); - let data_dir = tmpdir.path().join("data"); - std::fs::create_dir_all(&data_dir).unwrap(); - std::fs::write(data_dir.join("root"), HELLOWORLD_BLOB_CONTENTS).unwrap(); + std::fs::write(tmpdir.path().join("root"), HELLOWORLD_BLOB_CONTENTS).unwrap(); - let mut blob_service = gen_blob_service(tmpdir.path()); + let mut blob_service = gen_blob_service(); let root_node = import_path( &mut blob_service, - &mut gen_chunk_service(tmpdir.path()), - &mut gen_directory_service(tmpdir.path()), - data_dir.join("root"), + &mut gen_chunk_service(), + &mut gen_directory_service(), + tmpdir.path().join("root"), ) .expect("must succeed"); @@ -76,34 +77,35 @@ fn single_file() { fn complicated() { let tmpdir = TempDir::new().unwrap(); - let data_dir = tmpdir.path().join("data"); - - // Populate path to import - std::fs::create_dir_all(&data_dir).unwrap(); // File ``.keep` - std::fs::write(data_dir.join(".keep"), vec![]).unwrap(); + std::fs::write(tmpdir.path().join(".keep"), vec![]).unwrap(); // Symlink `aa` - std::os::unix::fs::symlink("/nix/store/somewhereelse", data_dir.join("aa")).unwrap(); + std::os::unix::fs::symlink("/nix/store/somewhereelse", tmpdir.path().join("aa")).unwrap(); // Directory `keep` - std::fs::create_dir(data_dir.join("keep")).unwrap(); + std::fs::create_dir(tmpdir.path().join("keep")).unwrap(); // File ``keep/.keep` - std::fs::write(data_dir.join("keep").join(".keep"), vec![]).unwrap(); + std::fs::write(tmpdir.path().join("keep").join(".keep"), vec![]).unwrap(); - let mut blob_service = gen_blob_service(tmpdir.path()); - let mut directory_service = gen_directory_service(tmpdir.path()); + let mut blob_service = gen_blob_service(); + let mut directory_service = gen_directory_service(); let root_node = import_path( &mut blob_service, - &mut gen_chunk_service(tmpdir.path()), + &mut gen_chunk_service(), &mut directory_service, - data_dir, + tmpdir.path(), ) .expect("must succeed"); // ensure root_node matched expectations assert_eq!( crate::proto::node::Node::Directory(proto::DirectoryNode { - name: "data".to_string(), + name: tmpdir + .path() + .file_name() + .unwrap() + .to_string_lossy() + .to_string(), digest: DIRECTORY_COMPLICATED.digest(), size: DIRECTORY_COMPLICATED.size(), }), diff --git a/tvix/store/src/tests/nar_renderer.rs b/tvix/store/src/tests/nar_renderer.rs index 8c0f738b5143..2b5b4f705c64 100644 --- a/tvix/store/src/tests/nar_renderer.rs +++ b/tvix/store/src/tests/nar_renderer.rs @@ -8,15 +8,13 @@ use crate::proto::FileNode; use crate::proto::SymlinkNode; use crate::tests::fixtures::*; use crate::tests::utils::*; -use tempfile::TempDir; #[test] fn single_symlink() { - let tmpdir = TempDir::new().unwrap(); let renderer = NARRenderer::new( - gen_blob_service(tmpdir.path()), - gen_chunk_service(tmpdir.path()), - gen_directory_service(tmpdir.path()), + gen_blob_service(), + gen_chunk_service(), + gen_directory_service(), ); // don't put anything in the stores, as we don't actually do any requests. @@ -39,15 +37,10 @@ fn single_symlink() { /// match what's in the store. #[test] fn single_file_missing_blob() { - let tmpdir = TempDir::new().unwrap(); - - let blob_service = gen_blob_service(tmpdir.path()); - let chunk_service = gen_chunk_service(tmpdir.path()); - let renderer = NARRenderer::new( - blob_service, - chunk_service, - gen_directory_service(tmpdir.path()), + gen_blob_service(), + gen_chunk_service(), + gen_directory_service(), ); let mut buf: Vec = vec![]; @@ -74,10 +67,8 @@ fn single_file_missing_blob() { /// than specified in the proto node. #[test] fn single_file_wrong_blob_size() { - let tmpdir = TempDir::new().unwrap(); - - let blob_service = gen_blob_service(tmpdir.path()); - let chunk_service = gen_chunk_service(tmpdir.path()); + let blob_service = gen_blob_service(); + let chunk_service = gen_chunk_service(); // insert blob and chunk into the stores chunk_service @@ -97,11 +88,7 @@ fn single_file_wrong_blob_size() { ) .unwrap(); - let renderer = NARRenderer::new( - blob_service, - chunk_service, - gen_directory_service(tmpdir.path()), - ); + let renderer = NARRenderer::new(blob_service, chunk_service, gen_directory_service()); let mut buf: Vec = vec![]; let e = renderer @@ -138,10 +125,8 @@ fn single_file_wrong_blob_size() { #[test] fn single_file() { - let tmpdir = TempDir::new().unwrap(); - - let blob_service = gen_blob_service(tmpdir.path()); - let chunk_service = gen_chunk_service(tmpdir.path()); + let blob_service = gen_blob_service(); + let chunk_service = gen_chunk_service(); chunk_service .put(HELLOWORLD_BLOB_CONTENTS.to_vec()) @@ -160,11 +145,7 @@ fn single_file() { ) .unwrap(); - let renderer = NARRenderer::new( - blob_service, - chunk_service, - gen_directory_service(tmpdir.path()), - ); + let renderer = NARRenderer::new(blob_service, chunk_service, gen_directory_service()); let mut buf: Vec = vec![]; renderer @@ -184,11 +165,9 @@ fn single_file() { #[test] fn test_complicated() { - let tmpdir = TempDir::new().unwrap(); - - let blob_service = gen_blob_service(tmpdir.path()); - let chunk_service = gen_chunk_service(tmpdir.path()); - let directory_service = gen_directory_service(tmpdir.path()); + let blob_service = gen_blob_service(); + let chunk_service = gen_chunk_service(); + let directory_service = gen_directory_service(); // put all data into the stores. let digest = chunk_service.put(EMPTY_BLOB_CONTENTS.to_vec()).unwrap(); diff --git a/tvix/store/src/tests/utils.rs b/tvix/store/src/tests/utils.rs index 4ddd4102fb97..9da9d2643b81 100644 --- a/tvix/store/src/tests/utils.rs +++ b/tvix/store/src/tests/utils.rs @@ -1,5 +1,3 @@ -use std::path::Path; - use crate::{ blobservice::{BlobService, SledBlobService}, chunkservice::{ChunkService, SledChunkService}, @@ -7,18 +5,18 @@ use crate::{ pathinfoservice::{PathInfoService, SledPathInfoService}, }; -pub fn gen_blob_service(p: &Path) -> impl BlobService + Send + Sync + Clone + 'static { - SledBlobService::new(p.join("blobs")).unwrap() +pub fn gen_blob_service() -> impl BlobService + Send + Sync + Clone + 'static { + SledBlobService::new_temporary().unwrap() } -pub fn gen_chunk_service(p: &Path) -> impl ChunkService + Clone { - SledChunkService::new(p.join("chunks")).unwrap() +pub fn gen_chunk_service() -> impl ChunkService + Clone { + SledChunkService::new_temporary().unwrap() } -pub fn gen_directory_service(p: &Path) -> impl DirectoryService + Send + Sync + Clone + 'static { - SledDirectoryService::new(p.join("directories")).unwrap() +pub fn gen_directory_service() -> impl DirectoryService + Send + Sync + Clone + 'static { + SledDirectoryService::new_temporary().unwrap() } -pub fn gen_pathinfo_service(p: &Path) -> impl PathInfoService { - SledPathInfoService::new(p.join("pathinfo")).unwrap() +pub fn gen_pathinfo_service() -> impl PathInfoService { + SledPathInfoService::new_temporary().unwrap() } -- cgit 1.4.1