mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(fuse,core): retry a create at once when a write replaced its blob (B185)
This commit is contained in:
1 parent
93f02672e6
commit
d26dc8ada4
6 files changed
+80
-11
No files matched your search
@@ -1477,22 +1477,28 @@ impl Db {
|
||||
/// Make a create due again if it was renamed or moved
|
||||
/// ([`Db::rewrite_op_target`]) while the attempt that just failed was on
|
||||
/// the wire, `parent_uid` and `name` being what that attempt sent. What it
|
||||
/// failed on may have been the target it no longer has. Returns whether it
|
||||
/// failed on may have been the target it no longer has. Likewise if a
|
||||
/// write replaced its blob (`blob_path` being the one the attempt read):
|
||||
/// the old one is discarded as the new one attaches, and an attempt that
|
||||
/// had not opened it yet failed on a file that is gone. Returns whether it
|
||||
/// was.
|
||||
pub fn retry_if_retargeted(
|
||||
&self,
|
||||
id: i64,
|
||||
parent_uid: Option<&str>,
|
||||
name: Option<&str>,
|
||||
blob_path: Option<&str>,
|
||||
) -> Result<bool> {
|
||||
let conn = self.conn.lock();
|
||||
let n = conn.execute(
|
||||
&format!(
|
||||
"UPDATE pending_op SET next_attempt_at = 0
|
||||
WHERE id = ?1 AND kind IN (?4, ?5) AND next_attempt_at < ?6
|
||||
AND ({PARENT_NOW} IS NOT ?2 OR name IS NOT ?3)"
|
||||
AND ({PARENT_NOW} IS NOT ?2 OR name IS NOT ?3 OR blob_path IS NOT ?7)"
|
||||
),
|
||||
params![id, parent_uid, name, OP_CREATE, OP_MKDIR, PARK_UNTIL],
|
||||
params![
|
||||
id, parent_uid, name, OP_CREATE, OP_MKDIR, PARK_UNTIL, blob_path
|
||||
],
|
||||
)?;
|
||||
Ok(n > 0)
|
||||
}
|
||||
|
||||
@@ -2969,7 +2969,8 @@ fn renaming_a_queued_create_rewrites_its_target() {
|
||||
|
||||
/// A create that failed on a name held by a queued change waits out a
|
||||
/// backoff. Renamed away from that name, it is due again at once; parked, it
|
||||
/// stays parked.
|
||||
/// stays parked. One whose blob a write replaced as it failed is due again at
|
||||
/// once too.
|
||||
#[test]
|
||||
fn renaming_a_backing_off_create_makes_it_due() {
|
||||
let db = Db::open_in_memory().unwrap();
|
||||
@@ -3011,8 +3012,13 @@ fn renaming_a_backing_off_create_makes_it_due() {
|
||||
db.record_op_failure(sent.id, "d is still held", 10_000)
|
||||
.unwrap();
|
||||
assert!(
|
||||
db.retry_if_retargeted(sent.id, sent.parent_uid.as_deref(), sent.name.as_deref())
|
||||
.unwrap()
|
||||
db.retry_if_retargeted(
|
||||
sent.id,
|
||||
sent.parent_uid.as_deref(),
|
||||
sent.name.as_deref(),
|
||||
sent.blob_path.as_deref()
|
||||
)
|
||||
.unwrap()
|
||||
);
|
||||
let ops = db.pending_ops().unwrap();
|
||||
assert_eq!(
|
||||
@@ -3023,8 +3029,42 @@ fn renaming_a_backing_off_create_makes_it_due() {
|
||||
0
|
||||
);
|
||||
assert!(
|
||||
!db.retry_if_retargeted(sent.id, sent.parent_uid.as_deref(), Some("f"))
|
||||
.unwrap()
|
||||
!db.retry_if_retargeted(
|
||||
sent.id,
|
||||
sent.parent_uid.as_deref(),
|
||||
Some("f"),
|
||||
sent.blob_path.as_deref()
|
||||
)
|
||||
.unwrap()
|
||||
);
|
||||
|
||||
// A write replaced the blob before the attempt opened it: the old one was
|
||||
// discarded, the attempt failed on a missing file, and the create is due
|
||||
// again at once with the new one (`docs/BUGS.md` B185).
|
||||
db.record_op_failure(sent.id, "No such file or directory", 10_000)
|
||||
.unwrap();
|
||||
db.attach_blob_to_create("local~failed", "/staging/newer", "{}")
|
||||
.unwrap()
|
||||
.expect("the create is still queued");
|
||||
db.record_op_failure(sent.id, "No such file or directory", 10_000)
|
||||
.unwrap();
|
||||
assert!(
|
||||
db.retry_if_retargeted(
|
||||
sent.id,
|
||||
sent.parent_uid.as_deref(),
|
||||
Some("f"),
|
||||
sent.blob_path.as_deref()
|
||||
)
|
||||
.unwrap()
|
||||
);
|
||||
assert!(
|
||||
!db.retry_if_retargeted(
|
||||
sent.id,
|
||||
sent.parent_uid.as_deref(),
|
||||
Some("f"),
|
||||
Some("/staging/newer")
|
||||
)
|
||||
.unwrap()
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -441,9 +441,10 @@ impl Core {
|
||||
op.id,
|
||||
op.parent_uid.as_deref(),
|
||||
op.name.as_deref(),
|
||||
op.blob_path.as_deref(),
|
||||
) {
|
||||
Ok(true) => {
|
||||
debug!(uid = %op.uid, "create was renamed while it failed; retrying")
|
||||
debug!(uid = %op.uid, "create was renamed or rewritten while it failed; retrying")
|
||||
}
|
||||
Ok(false) => {}
|
||||
Err(e) => {
|
||||
|
||||
@@ -12,6 +12,25 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B185 — A new file written again just as it starts uploading waits out a retry
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
**Found:** 2026-10-05, in the confirmation run on 3073052 (`one_client_on_a_good_link` seed 6,
|
||||
then `one_client_deleting_files_as_they_upload` seed 4). The queues did not drain in time: a
|
||||
create had failed with "No such file or directory" and was backing off.
|
||||
|
||||
**Where:** `Db::retry_if_retargeted` in `crates/pdfs-core/src/db/ops.rs`, called from the drain
|
||||
in `crates/pdfs-fuse/src/drain.rs`.
|
||||
|
||||
**Cause.** A write to a file whose create is queued attaches a new blob to the create and
|
||||
discards the old one at once. An attempt that had claimed the create but not yet opened the old
|
||||
blob failed on a file that was gone, and backed off as if Drive had refused it. The new blob was
|
||||
ready, but the create waited out the backoff.
|
||||
|
||||
**Fix.** A failed create whose blob was replaced while it ran is due again at once, the way one
|
||||
renamed while it ran already was. Test `renaming_a_backing_off_create_makes_it_due` in
|
||||
`crates/pdfs-core/src/db/tests.rs`.
|
||||
|
||||
## B184 — A file renamed after its create lost its answer stays on Drive under its old name
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
|
||||
@@ -105,6 +105,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
||||
- **A file renamed while the connection lost Drive's answer to its upload lands once.** It no
|
||||
longer stayed on Drive under its old name too, or, when deleted, under its old name alone
|
||||
(B184).
|
||||
- **A new file written again just as it starts uploading no longer waits out a retry** before
|
||||
it reaches Drive (B185).
|
||||
- **A file renamed just as its upload finished keeps its new name.** The mount no longer showed
|
||||
the old name, and a second rename no longer failed with "No such file or directory" (B152,
|
||||
B172).
|
||||
|
||||
@@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
- **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0).
|
||||
- **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded
|
||||
locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c).
|
||||
- Fixes from the simulation and account runs since then: B125, B127, B130 and B141 to B184.
|
||||
- Fixes from the simulation and account runs since then: B125, B127, B130 and B141 to B185.
|
||||
- **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app,
|
||||
tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`; Discard button and
|
||||
`pdfs sync discard`; a warning badge on the node in the file browser.
|
||||
@@ -110,7 +110,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
after restarts: a write over a create never read back (B182) and a trash that forgot
|
||||
what its create sent (B183). Replays with those fixed found a third: a create renamed
|
||||
after it lost its answer (B184). All three fixed; no known-bug matcher fired. It was
|
||||
stopped before `three_clients`.
|
||||
stopped before `three_clients`. The rerun on 3073052 found a create that waited out
|
||||
a retry after a write replaced its blob (B185, fixed).
|
||||
- [ ] A clean account run. The one on a6f36d5 (2026-10-04, LAN: 427 passed, 12 failed)
|
||||
failed B113 and B121 on every on-demand mount, two mirror-to-mirror move cases, and
|
||||
the journal check. B121 was a bug (B149, fixed); B113's case still expected creates
|
||||
|
||||
Reference in new issue
Block a user