refactor(tvix/castore/blobsvc): return Box, not Arc

While we currently mostly use it in an Arc, as we need to clone it
inside PathInfoService, there might be other usecases not requiring it
to be Clone.

Change-Id: I7bd337cd2e4c2d4154b385461eefa62c9b78345d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10482
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This commit is contained in:
Florian Klink 2023-12-31 16:13:29 +02:00 committed by clbot
parent e2b6c77bfc
commit 9ca1353122
3 changed files with 18 additions and 12 deletions

View file

@ -1,4 +1,3 @@
use std::sync::Arc;
use url::Url; use url::Url;
use crate::{proto::blob_service_client::BlobServiceClient, Error}; use crate::{proto::blob_service_client::BlobServiceClient, Error};
@ -16,7 +15,7 @@ use super::{
/// - `simplefs://` ([SimpleFilesystemBlobService]) /// - `simplefs://` ([SimpleFilesystemBlobService])
/// ///
/// See their `from_url` methods for more details about their syntax. /// See their `from_url` methods for more details about their syntax.
pub async fn from_addr(uri: &str) -> Result<Arc<dyn BlobService>, crate::Error> { pub async fn from_addr(uri: &str) -> Result<Box<dyn BlobService>, crate::Error> {
let url = Url::parse(uri) let url = Url::parse(uri)
.map_err(|e| crate::Error::StorageError(format!("unable to parse url: {}", e)))?; .map_err(|e| crate::Error::StorageError(format!("unable to parse url: {}", e)))?;
@ -25,7 +24,7 @@ pub async fn from_addr(uri: &str) -> Result<Arc<dyn BlobService>, crate::Error>
if url.has_host() || !url.path().is_empty() { if url.has_host() || !url.path().is_empty() {
return Err(Error::StorageError("invalid url".to_string())); return Err(Error::StorageError("invalid url".to_string()));
} }
Arc::new(MemoryBlobService::default()) Box::<MemoryBlobService>::default()
} else if url.scheme() == "sled" { } else if url.scheme() == "sled" {
// sled doesn't support host, and a path can be provided (otherwise // sled doesn't support host, and a path can be provided (otherwise
// it'll live in memory only). // it'll live in memory only).
@ -42,11 +41,11 @@ pub async fn from_addr(uri: &str) -> Result<Arc<dyn BlobService>, crate::Error>
// TODO: expose other parameters as URL parameters? // TODO: expose other parameters as URL parameters?
if url.path().is_empty() { if url.path().is_empty() {
return Ok(Arc::new( return Ok(Box::new(
SledBlobService::new_temporary().map_err(|e| Error::StorageError(e.to_string()))?, SledBlobService::new_temporary().map_err(|e| Error::StorageError(e.to_string()))?,
)); ));
} }
return Ok(Arc::new( return Ok(Box::new(
SledBlobService::new(url.path()).map_err(|e| Error::StorageError(e.to_string()))?, SledBlobService::new(url.path()).map_err(|e| Error::StorageError(e.to_string()))?,
)); ));
} else if url.scheme().starts_with("grpc+") { } else if url.scheme().starts_with("grpc+") {
@ -56,13 +55,13 @@ pub async fn from_addr(uri: &str) -> Result<Arc<dyn BlobService>, crate::Error>
// - In the case of non-unix sockets, there must be a host, but no path. // - In the case of non-unix sockets, there must be a host, but no path.
// Constructing the channel is handled by tvix_castore::channel::from_url. // Constructing the channel is handled by tvix_castore::channel::from_url.
let client = BlobServiceClient::new(crate::tonic::channel_from_url(&url).await?); let client = BlobServiceClient::new(crate::tonic::channel_from_url(&url).await?);
Arc::new(GRPCBlobService::from_client(client)) Box::new(GRPCBlobService::from_client(client))
} else if url.scheme() == "simplefs" { } else if url.scheme() == "simplefs" {
if url.path().is_empty() { if url.path().is_empty() {
return Err(Error::StorageError("Invalid filesystem path".to_string())); return Err(Error::StorageError("Invalid filesystem path".to_string()));
} }
Arc::new(SimpleFilesystemBlobService::new(url.path().into()).await?) Box::new(SimpleFilesystemBlobService::new(url.path().into()).await?)
} else { } else {
Err(crate::Error::StorageError(format!( Err(crate::Error::StorageError(format!(
"unknown scheme: {}", "unknown scheme: {}",

View file

@ -77,7 +77,9 @@ async fn construct_services(
Arc<dyn DirectoryService>, Arc<dyn DirectoryService>,
Box<dyn PathInfoService>, Box<dyn PathInfoService>,
)> { )> {
let blob_service = blobservice::from_addr(blob_service_addr.as_ref()).await?; let blob_service: Arc<dyn BlobService> = blobservice::from_addr(blob_service_addr.as_ref())
.await?
.into();
let directory_service = directoryservice::from_addr(directory_service_addr.as_ref()).await?; let directory_service = directoryservice::from_addr(directory_service_addr.as_ref()).await?;
let path_info_service = pathinfoservice::from_addr( let path_info_service = pathinfoservice::from_addr(
path_info_service_addr.as_ref(), path_info_service_addr.as_ref(),

View file

@ -12,6 +12,7 @@ use tokio_listener::SystemOptions;
use tokio_listener::UserOptions; use tokio_listener::UserOptions;
use tracing_subscriber::prelude::*; use tracing_subscriber::prelude::*;
use tvix_castore::blobservice; use tvix_castore::blobservice;
use tvix_castore::blobservice::BlobService;
use tvix_castore::directoryservice; use tvix_castore::directoryservice;
use tvix_castore::import; use tvix_castore::import;
use tvix_castore::proto::blob_service_server::BlobServiceServer; use tvix_castore::proto::blob_service_server::BlobServiceServer;
@ -194,7 +195,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
path_info_service_addr, path_info_service_addr,
} => { } => {
// initialize stores // initialize stores
let blob_service = blobservice::from_addr(&blob_service_addr).await?; let blob_service: Arc<dyn BlobService> =
blobservice::from_addr(&blob_service_addr).await?.into();
let directory_service = directoryservice::from_addr(&directory_service_addr).await?; let directory_service = directoryservice::from_addr(&directory_service_addr).await?;
let path_info_service = pathinfoservice::from_addr( let path_info_service = pathinfoservice::from_addr(
&path_info_service_addr, &path_info_service_addr,
@ -249,7 +251,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
path_info_service_addr, path_info_service_addr,
} => { } => {
// FUTUREWORK: allow flat for single files? // FUTUREWORK: allow flat for single files?
let blob_service = blobservice::from_addr(&blob_service_addr).await?; let blob_service: Arc<dyn BlobService> =
blobservice::from_addr(&blob_service_addr).await?.into();
let directory_service = directoryservice::from_addr(&directory_service_addr).await?; let directory_service = directoryservice::from_addr(&directory_service_addr).await?;
let path_info_service = pathinfoservice::from_addr( let path_info_service = pathinfoservice::from_addr(
&path_info_service_addr, &path_info_service_addr,
@ -351,7 +354,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
list_root, list_root,
threads, threads,
} => { } => {
let blob_service = blobservice::from_addr(&blob_service_addr).await?; let blob_service: Arc<dyn BlobService> =
blobservice::from_addr(&blob_service_addr).await?.into();
let directory_service = directoryservice::from_addr(&directory_service_addr).await?; let directory_service = directoryservice::from_addr(&directory_service_addr).await?;
let path_info_service = pathinfoservice::from_addr( let path_info_service = pathinfoservice::from_addr(
&path_info_service_addr, &path_info_service_addr,
@ -395,7 +399,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
path_info_service_addr, path_info_service_addr,
list_root, list_root,
} => { } => {
let blob_service = blobservice::from_addr(&blob_service_addr).await?; let blob_service: Arc<dyn BlobService> =
blobservice::from_addr(&blob_service_addr).await?.into();
let directory_service = directoryservice::from_addr(&directory_service_addr).await?; let directory_service = directoryservice::from_addr(&directory_service_addr).await?;
let path_info_service = pathinfoservice::from_addr( let path_info_service = pathinfoservice::from_addr(
&path_info_service_addr, &path_info_service_addr,