blaginin commented on code in PR #899:
URL:
https://github.com/apache/arrow-rs-object-store/pull/899#discussion_r4446440566
##########
src/local.rs:
##########
@@ -1100,21 +1101,112 @@ struct LocalUpload {
#[derive(Debug)]
struct UploadState {
dest: PathBuf,
- file: Mutex<Option<File>>,
+ file: Mutex<Option<StagedUpload>>,
+}
+
+#[derive(Debug)]
+struct StagedUpload {
+ file: Option<File>,
+ path: Option<PathBuf>,
+}
+
+impl StagedUpload {
+ fn abort(&mut self) -> Result<()> {
+ // Close before unlinking, including on platforms that cannot unlink
open files.
+ drop(self.file.take());
+ if let Some(path) = &self.path {
+ std::fs::remove_file(path).map_err(|source|
Error::UnableToDeleteFile {
+ source,
+ path: path.clone(),
+ })?;
+ self.path = None;
+ }
+ Ok(())
+ }
+}
+
+impl Drop for StagedUpload {
+ fn drop(&mut self) {
+ let _ = self.abort();
+ }
+}
+
+impl UploadState {
+ fn abort(&self) -> Result<()> {
+ if let Some(mut staged) = self.file.lock().take() {
+ staged.abort()?;
+ }
+ Ok(())
+ }
}
impl LocalUpload {
pub(crate) fn new(src: PathBuf, dest: PathBuf, file: File, fsync: bool) ->
Self {
Self {
+ mode: MultipartUploadMode::Overwrite,
state: Arc::new(UploadState {
dest,
- file: Mutex::new(Some(file)),
+ file: Mutex::new(Some(StagedUpload {
+ file: Some(file),
+ path: Some(src),
+ })),
}),
- src: Some(src),
offset: 0,
fsync,
}
}
+
+ async fn publish(
Review Comment:
somewhat related change... @asuresh8 noticed:
> In LocalUpload, if the rename succeeds but the directory fsync fails, src
stays set, so a later abort or drop acts on a path that has already been
renamed away.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
