From da6cbb4a459d02111c44a67d3d0dd7e654abff23 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Wed, 13 Sep 2023 14:20:21 +0200
Subject: refactor(tvix/store/blobsvc): make BlobStore async

We previously kept the trait of a BlobService sync.

This however had some annoying consequences:

 - It became more and more complicated to track when we're in a context
   with an async runtime in the context or not, producing bugs like
   https://b.tvl.fyi/issues/304
 - The sync trait shielded away async clients from async worloads,
   requiring manual block_on code inside the gRPC client code, and
   spawn_blocking calls in consumers of the trait, even if they were
   async (like the gRPC server)
 - We had to write our own custom glue code (SyncReadIntoAsyncRead)
   to convert a sync io::Read into a tokio::io::AsyncRead, which already
   existed in tokio internally, but upstream ia hesitant to expose.

This now makes the BlobService trait async (via the async_trait macro,
like we already do in various gRPC parts), and replaces the sync readers
and writers with their async counterparts.

Tests interacting with a BlobService now need to have an async runtime
available, the easiest way for this is to mark the test functions
with the tokio::test macro, allowing us to directly .await in the test
function.

In places where we don't have an async runtime available from context
(like tvix-cli), we can pass one down explicitly.

Now that we don't provide a sync interface anymore, the (sync) FUSE
library now holds a pointer to a tokio runtime handle, and needs to at
least have 2 threads available when talking to a blob service (which is
why some of the tests now use the multi_thread flavor).

The FUSE tests got a bit more verbose, as we couldn't use the
setup_and_mount function accepting a callback anymore. We can hopefully
move some of the test fixture setup to rstest in the future to make this
less repetitive.

Co-Authored-By: Connor Brewster <cbrewster@hey.com>
Change-Id: Ia0501b606e32c852d0108de9c9016b21c94a3c05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9329
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
---
 tvix/store/src/bin/tvix-store.rs | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

(limited to 'tvix/store/src/bin/tvix-store.rs')

diff --git a/tvix/store/src/bin/tvix-store.rs b/tvix/store/src/bin/tvix-store.rs
index f69c2f7fab..0da264808e 100644
--- a/tvix/store/src/bin/tvix-store.rs
+++ b/tvix/store/src/bin/tvix-store.rs
@@ -6,6 +6,7 @@ use nix_compat::store_path::StorePath;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
+use tokio::task::JoinHandle;
 use tracing_subscriber::prelude::*;
 use tvix_store::blobservice;
 use tvix_store::directoryservice;
@@ -199,17 +200,24 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
                     let directory_service = directory_service.clone();
                     let path_info_service = path_info_service.clone();
 
-                    let task = tokio::task::spawn_blocking(move || -> io::Result<()> {
+                    let task: JoinHandle<io::Result<()>> = tokio::task::spawn(async move {
                         // Ingest the path into blob and directory service.
                         let root_node = import::ingest_path(
                             blob_service.clone(),
                             directory_service.clone(),
                             &path,
                         )
+                        .await
                         .expect("failed to ingest path");
 
                         // Ask the PathInfoService for the NAR size and sha256
-                        let (nar_size, nar_sha256) = path_info_service.calculate_nar(&root_node)?;
+                        let root_node_copy = root_node.clone();
+                        let path_info_service_clone = path_info_service.clone();
+                        let (nar_size, nar_sha256) = tokio::task::spawn_blocking(move || {
+                            path_info_service_clone.calculate_nar(&root_node_copy)
+                        })
+                        .await
+                        .unwrap()?;
 
                         // TODO: make a path_to_name helper function?
                         let name = path
@@ -241,7 +249,10 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
 
                         // put into [PathInfoService], and return the PathInfo that we get back
                         // from there (it might contain additional signatures).
-                        let path_info = path_info_service.put(path_info)?;
+                        let path_info =
+                            tokio::task::spawn_blocking(move || path_info_service.put(path_info))
+                                .await
+                                .unwrap()?;
 
                         let node = path_info.node.unwrap().node.unwrap();
 
@@ -304,9 +315,9 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
             // task.
             tokio::task::spawn_blocking(move || -> io::Result<()> {
                 info!("mounting tvix-store on {:?}", fuse_session.mountpoint());
-                let res = fuse_session.run()?;
+                fuse_session.run()?;
                 info!("unmount occured, terminating…");
-                Ok(res)
+                Ok(())
             })
             .await??;
         }
-- 
cgit 1.4.1