You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kevinjqliu (via GitHub)" <gi...@apache.org> on 2026/06/17 15:30:11 UTC
[PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
kevinjqliu opened a new pull request, #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758
Follow-up to #643.
rename_if_not_exists uses RenameTargetMode::Create, which is implemented as copy-then-delete. The copy side now goes through the fsync-aware path, but the source delete also needs to sync the source parent directory when with_fsync(true) is enabled so the source-name removal is durable before rename_opts returns.
This keeps normal delete_stream behavior unchanged and only passes self.fsync for the internal delete that is part of create-mode rename.
Validation:
- cargo test -p object_store fsync_rename_if_not_exists_propagates_source_delete_sync_error
- cargo test -p object_store local::tests::
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#issuecomment-4732314021
cc @Barre this is a follow up to #643 do you mind taking a look and see if it makes sense directionally?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#discussion_r3435099305
##########
src/local.rs:
##########
@@ -513,7 +513,7 @@ impl ObjectStore for LocalFileSystem {
let config = Arc::clone(&config);
maybe_spawn_blocking(move || {
let location = location?;
- Self::delete_location(config, automatic_cleanup, &location)?;
+ Self::delete_location(config, automatic_cleanup, &location, false)?;
Review Comment:
WHy doesn't fsync apply to standalone deletes (if we don't fsync isn't there some chance that a deleted file will get "undeleted" on crash)?
This code is no worse than main so we can merge it in, but I don't fully understand. This can be a follow on 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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#discussion_r3431209157
##########
src/local.rs:
##########
@@ -524,7 +513,10 @@ impl ObjectStore for LocalFileSystem {
let config = Arc::clone(&config);
maybe_spawn_blocking(move || {
let location = location?;
- Self::delete_location(config, automatic_cleanup, &location)?;
+ // Do not apply `with_fsync` to standalone deletes.
+ // Use `fsync=true` only when removing the source after a synced copy in
+ // `rename_if_not_exists`.
Review Comment:
```suggestion
// `with_fsync` does not apply to standalone deletes; only create-mode rename
// fsyncs its internal source removal as part of the durable copy-and-delete.
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#discussion_r3436781999
##########
src/local.rs:
##########
@@ -513,7 +513,7 @@ impl ObjectStore for LocalFileSystem {
let config = Arc::clone(&config);
maybe_spawn_blocking(move || {
let location = location?;
- Self::delete_location(config, automatic_cleanup, &location)?;
+ Self::delete_location(config, automatic_cleanup, &location, false)?;
Review Comment:
created tracking issue https://github.com/apache/arrow-rs-object-store/issues/767
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#discussion_r3430925121
##########
src/local.rs:
##########
@@ -513,7 +513,7 @@ impl ObjectStore for LocalFileSystem {
let config = Arc::clone(&config);
maybe_spawn_blocking(move || {
let location = location?;
- Self::delete_location(config, automatic_cleanup, &location)?;
+ Self::delete_location(config, automatic_cleanup, &location, false)?;
Review Comment:
why doesn't this need to `fsync`? Maybe we can add a comment explaining why
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758#discussion_r3436778013
##########
src/local.rs:
##########
@@ -513,7 +513,7 @@ impl ObjectStore for LocalFileSystem {
let config = Arc::clone(&config);
maybe_spawn_blocking(move || {
let location = location?;
- Self::delete_location(config, automatic_cleanup, &location)?;
+ Self::delete_location(config, automatic_cleanup, &location, false)?;
Review Comment:
> WHy doesn't fsync apply to standalone deletes (if we don't fsync isn't there some chance that a deleted file will get "undeleted" on crash)?
it should! but it doesnt right now, the documented mentions only write-like operations and not deletes
https://github.com/apache/arrow-rs-object-store/blob/c9db3e22d2eed690b7a9c14a988ec85d6b9304ad/src/local.rs#L310-L316
Let's do a follow up to make standalone `delete` / `delete_stream` crash-durable
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(local): fsync create-mode rename source delete [arrow-rs-object-store]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #758:
URL: https://github.com/apache/arrow-rs-object-store/pull/758
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org