Unverified Commit e3f4a2cd authored by Jaromir Hamala's avatar Jaromir Hamala Committed by GitHub
Browse files

fix(s3s-fs): make metadata file writes atomic (#360) 

Resolves a race condition where load_internal_info() could
attempt to parse a metadata file after its creation, but
before its contents were written by save_internal_info().
This would cause a deserialization error on an empty file.

This change makes the write operation in save_internal_info()
atomic by using a temporary file and renaming it to the final
destination.

I applied the same fix to two other functions that used non-atomic
writes even I did not observe them causing issues in our tests.

This builds on the work in https://github.com/s3s-project/s3s/pull/117.

Relevant Error Log:
ERROR s3s_fs_internal_error: span trace:
location: crates/s3s-fs/src/fs.rs:129:19, error: EOF while parsing a value at line 1 column 0
parent 6b861cb3

crates/s3s-fs/src/fs.rs

+13 −4
Original line number Diff line number Diff line
@@ -14,7 +14,7 @@ use std::sync::atomic::{AtomicU64, Ordering}; 


use tokio::fs;

use tokio::fs::File;

use tokio::io::{AsyncReadExt, BufWriter};

use tokio::io::{AsyncReadExt, AsyncWriteExt, BufWriter};



use path_absolutize::Absolutize;

use uuid::Uuid;
@@ -109,7 +109,10 @@ impl FileSystem { 
    ) -> Result<()> {

        let path = self.get_metadata_path(bucket, key, upload_id)?;

        let content = serde_json::to_vec(metadata)?;

        fs::write(&path, &content).await?;

        let mut file_writer = self.prepare_file_write(&path).await?;

        file_writer.writer().write_all(&content).await?;

        file_writer.writer().flush().await?;

        file_writer.done().await?;

        Ok(())

    }
@@ -133,7 +136,10 @@ impl FileSystem { 
    pub(crate) async fn save_internal_info(&self, bucket: &str, key: &str, info: &InternalInfo) -> Result<()> {

        let path = self.get_internal_info_path(bucket, key)?;

        let content = serde_json::to_vec(info)?;

        fs::write(&path, &content).await?;

        let mut file_writer = self.prepare_file_write(&path).await?;

        file_writer.writer().write_all(&content).await?;

        file_writer.writer().flush().await?;

        file_writer.done().await?;

        Ok(())

    }
@@ -164,7 +170,10 @@ impl FileSystem { 
        let ak: Option<&str> = cred.map(|c| c.access_key.as_str());



        let content = serde_json::to_vec(&ak)?;

        fs::write(&upload_info_path, &content).await?;

        let mut file_writer = self.prepare_file_write(&upload_info_path).await?;

        file_writer.writer().write_all(&content).await?;

        file_writer.writer().flush().await?;

        file_writer.done().await?;



        Ok(upload_id)

    }