jgiannuzzi commented on code in PR #676:
URL:
https://github.com/apache/arrow-rs-object-store/pull/676#discussion_r3046493043
##########
src/local.rs:
##########
@@ -1913,4 +1956,16 @@ mod unix_test {
spawned.await.unwrap();
}
+
+ #[test]
+ fn test_close_file_detects_error() {
+ use std::os::unix::io::AsRawFd;
+
+ let file = tempfile::tempfile().unwrap();
+ let fd = file.as_raw_fd();
+ // Close the fd behind Rust's back so close_file will get EBADF
+ assert_eq!(unsafe { libc::close(fd) }, 0);
+ let err = super::close_file(file).unwrap_err();
+ assert_eq!(err.raw_os_error(), Some(libc::EBADF));
+ }
Review Comment:
This test is just validating our logic by emulating a failure case.
The actual failure that can happen is when using a remote file system (e.g.
NFS) and suffering from an underlying issue with the mount. In this case, the
`close` could fail because some previous queued writes failed and checking the
`close` return code allows the calling application to know that something
failed, _even if `fsync` wasn't called_.
See this comment on the Polars issue for more details:
https://github.com/pola-rs/polars/issues/21002#issuecomment-3579381898
I don't think I claimed this was an alternative to `fsync`. It's rather a
way to make sure these errors are reported to the caller. For durability
guarantees, `fsync` remains required (and I think this should be part of
another PR).
--
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]
