Re: [pbs-devel] [PATCH proxmox-backup] api: chunk reader: make reading from filesystem fully async

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 26.11.25 um 17:28 schrieb Christian Ebner:
> Blocking the thread is problematic here and must be avoided, so
> read the chunk data via tokio::fs::read() instead of std::fs::read()
> and make the full loading from filesystem branch async.

Nothing against that, but "async" here comes a bit with a bigger asterisks,
as:

"This operation is implemented by running the equivalent blocking operation
on a separate thread pool using spawn_blocking."
-- https://docs.rs/tokio/latest/tokio/fs/fn.read.html

So technically async, but not really does any async IO (tokio io uring when? ;)).

The important thing is that it cannot block anything, so it _is_ an OK solution
here, might be nice to adapt the commit message slightly though, e.g. something
like:

...::read() to move the blocking file read in the "full loading from filesystem"
branch to it's own thread pool. Can be done on applying though.

> 
> Encountered while investigating a user provided backtrace looking for
> possible causes of hanging backups reported in [0].
> 
> [0] https://forum.proxmox.com/threads/176444/post-819858
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  src/api2/reader/mod.rs | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/api2/reader/mod.rs b/src/api2/reader/mod.rs
> index f7adc366f..1e74b0758 100644
> --- a/src/api2/reader/mod.rs
> +++ b/src/api2/reader/mod.rs
> @@ -321,7 +321,7 @@ fn download_chunk(
>          }
>  
>          let body = match &env.backend {
> -            DatastoreBackend::Filesystem => load_from_filesystem(env, &digest)?,
> +            DatastoreBackend::Filesystem => load_from_filesystem(env, &digest).await?,
>              DatastoreBackend::S3(s3_client) => match env.datastore.cache() {
>                  None => fetch_from_object_store(s3_client, &digest).await?,
>                  Some(cache) => {
> @@ -357,13 +357,14 @@ async fn fetch_from_object_store(s3_client: &S3Client, digest: &[u8; 32]) -> Res
>      bail!("cannot find chunk with digest {}", hex::encode(digest));
>  }
>  
> -fn load_from_filesystem(env: &ReaderEnvironment, digest: &[u8; 32]) -> Result<Body, Error> {
> +async fn load_from_filesystem(env: &ReaderEnvironment, digest: &[u8; 32]) -> Result<Body, Error> {
>      let (path, _) = env.datastore.chunk_path(digest);
>      let path2 = path.clone();
>  
>      env.debug(format!("download chunk {path:?}"));
>  
> -    let data = proxmox_async::runtime::block_in_place(|| std::fs::read(path))
> +    let data = tokio::fs::read(path)
> +        .await
>          .map_err(move |err| http_err!(BAD_REQUEST, "reading file {path2:?} failed: {err}"))?;
>      Ok(Body::from(data))
>  }



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel