Re: [PR] feat(services/hdfs): Support atomic write dir for HDFS [incubator-opendal]

["Xuanwo (via GitHub)" <gi...@apache.org>]

Xuanwo commented on code in PR #3763:
URL: https://github.com/apache/incubator-opendal/pull/3763#discussion_r1428640640


##########
core/src/services/hdfs/backend.rs:
##########
@@ -163,13 +194,35 @@ impl Builder for HdfsBuilder {
 pub struct HdfsBackend {
     root: String,
     client: Arc<hdrs::Client>,
+    atomic_write_dir: Option<PathBuf>,

Review Comment:
   We don't need to use `PathBuf` here since all hdfs path are required to be UTF-8.



##########
core/src/services/hdfs/backend.rs:
##########
@@ -262,12 +317,36 @@ impl Accessor for HdfsBackend {
             open_options.write(true);
         }
 
-        let f = open_options
-            .async_open(&p)
-            .await
-            .map_err(new_std_io_error)?;
+        if let Some(tmp_path) = &tmp_path {
+            let mut t = open_options
+                .async_open(tmp_path.to_str().unwrap())
+                .await
+                .map_err(new_std_io_error)?;
 
-        Ok((RpWrite::new(), HdfsWriter::new(f)))
+            let tmp_path = tmp_path.clone();
+            let target_path = target_path.clone();
+
+            t.flush().await.map_err(new_std_io_error)?;
+            t.close().await.map_err(new_std_io_error)?;
+
+            self.client
+                .rename_file(tmp_path.to_str().unwrap(), target_path.to_str().unwrap())

Review Comment:
   I didn't get this. Why we can't finish this in `HdfsWriter`?



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org