maurice/snix
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(tvix/store): drop calculate_nar from PathInfoService

Browse source 
This shouldn't be part of the PathInfoService trait.

Pretty much none of the PathInfoServices do implement it, and requiring
them to implement it means they also cannot make use of this calculation
already being done by other PathInfoServices.

Move it out into its own NarCalculationService trait, defined somewhere
at tvix_store::nar, and have everyone who wants to trigger nar
calculation use nar_calculation_service directly, which now is an
additional field in TvixStoreIO for example.

It being moved outside the PathInfoService trait doesn't prohibit
specific implementations to implement it (like the GRPC client for the
`PathInfoService` does.

This is currently wired together in a bit of a hacky fashion - as of
now, everything uses the naive implementation that traverses blob and
directoryservice, rather than composing it properly. I want to leave
that up to a later CL, dealing with other parts of store composition
too.

Change-Id: I18d07ea4301d4a07651b8218bc5fe95e4e307208
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11619
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This commit is contained in:
Florian Klink 2024-05-10 08:59:25 +03:00 • committed by clbot
parent 944a781354
commit 14766cfe1d
20 changed files with 241 additions and 187 deletions
Download patch file Download diff file Expand all files Collapse all files

25
tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs
View file

@ -1,4 +1,4 @@
use crate::nar::RenderError;
use crate::nar::{NarCalculationService, RenderError};
use crate::pathinfoservice::PathInfoService;
use crate::proto;
use futures::{stream::BoxStream, TryStreamExt};
@ -7,23 +7,26 @@ use tonic::{async_trait, Request, Response, Result, Status};
use tracing::{instrument, warn};
use tvix_castore::proto as castorepb;
pub struct GRPCPathInfoServiceWrapper<PS> {
inner: PS,
pub struct GRPCPathInfoServiceWrapper<PS, NS> {
path_info_service: PS,
// FUTUREWORK: allow exposing without allowing listing
nar_calculation_service: NS,
}
impl<PS> GRPCPathInfoServiceWrapper<PS> {
pub fn new(path_info_service: PS) -> Self {
impl<PS, NS> GRPCPathInfoServiceWrapper<PS, NS> {
pub fn new(path_info_service: PS, nar_calculation_service: NS) -> Self {
Self {
inner: path_info_service,
path_info_service,
nar_calculation_service,
}
}
}
#[async_trait]
impl<PS> proto::path_info_service_server::PathInfoService for GRPCPathInfoServiceWrapper<PS>
impl<PS, NS> proto::path_info_service_server::PathInfoService for GRPCPathInfoServiceWrapper<PS, NS>
where
PS: Deref<Target = dyn PathInfoService> + Send + Sync + 'static,
NS: NarCalculationService + Send + Sync + 'static,
{
type ListStream = BoxStream<'static, tonic::Result<proto::PathInfo, Status>>;
@ -39,7 +42,7 @@ where
.to_vec()
.try_into()
.map_err(|_e| Status::invalid_argument("invalid output digest length"))?;
match self.inner.get(digest).await {
match self.path_info_service.get(digest).await {
Ok(None) => Err(Status::not_found("PathInfo not found")),
Ok(Some(path_info)) => Ok(Response::new(path_info)),
Err(e) => {
@ -57,7 +60,7 @@ where
// Store the PathInfo in the client. Clients MUST validate the data
// they receive, so we don't validate additionally here.
match self.inner.put(path_info).await {
match self.path_info_service.put(path_info).await {
Ok(path_info_new) => Ok(Response::new(path_info_new)),
Err(e) => {
warn!(err = %e, "failed to put PathInfo");
@ -79,7 +82,7 @@ where
Err(Status::invalid_argument("invalid root node"))?
}
match self.inner.calculate_nar(&root_node).await {
match self.nar_calculation_service.calculate_nar(&root_node).await {
Ok((nar_size, nar_sha256)) => Ok(Response::new(proto::CalculateNarResponse {
nar_size,
nar_sha256: nar_sha256.to_vec().into(),
@ -99,7 +102,7 @@ where
_request: Request<proto::ListPathInfoRequest>,
) -> Result<Response<Self::ListStream>, Status> {
let stream = Box::pin(
self.inner
self.path_info_service
.list()
.map_err(|e| Status::internal(e.to_string())),
);