mirror of
https://github.com/narrrl/proton-drive-linux.git
synced 2026-11-03 16:41:26 +01:00
fix(core,fuse): a deleted file's trash is not held back by its create (B171)
A file deleted after its create was sent keeps the op as a trash. The op kept the create's attempts and backoff, and the drain recorded the failure of a create attempt still on the wire on the row that was by then the trash. The trash waited out the create's backoff. The trash starts due, as a create that becomes a revision does, and the drain records a failed attempt only if the op is still the kind it ran as (`Db::record_attempt_failure`).
This commit is contained in:
1 parent
2502af6e3c
commit
e32f72bfca
6 files changed
+116
-5
No files matched your search
@@ -151,6 +151,7 @@ fn drop_doomed_ops(tx: &rusqlite::Transaction<'_>, uid: &str) -> Result<Vec<Stri
|
||||
"{DOOMED}
|
||||
UPDATE pending_op
|
||||
SET kind = 'trash', issue = NULL,
|
||||
attempts = 0, last_error = NULL, next_attempt_at = 0,
|
||||
parent_uid = CASE WHEN {PARENT_NOW} LIKE '{LOCAL_VOLUME}~%' THEN NULL
|
||||
ELSE {PARENT_NOW} END,
|
||||
parent_lid = CASE WHEN {PARENT_NOW} LIKE '{LOCAL_VOLUME}~%' THEN NULL
|
||||
@@ -1276,6 +1277,31 @@ impl Db {
|
||||
/// A rename with a newer one queued behind it ([`Db::enqueue_op`]) is
|
||||
/// dropped instead: retried, it could land after the newer one and undo it.
|
||||
pub fn record_op_failure(&self, id: i64, error: &str, next_attempt_at: i64) -> Result<()> {
|
||||
self.record_failure(id, None, error, next_attempt_at)
|
||||
}
|
||||
|
||||
/// [`Db::record_op_failure`] for an attempt at op `id` as a `kind` op. An
|
||||
/// op that has become another kind while the attempt ran is left as it
|
||||
/// is: a create deleted on the wire is a trash now, and the create's
|
||||
/// failure would hold the trash back for the create's backoff
|
||||
/// (`docs/BUGS.md` B171).
|
||||
pub fn record_attempt_failure(
|
||||
&self,
|
||||
id: i64,
|
||||
kind: &str,
|
||||
error: &str,
|
||||
next_attempt_at: i64,
|
||||
) -> Result<()> {
|
||||
self.record_failure(id, Some(kind), error, next_attempt_at)
|
||||
}
|
||||
|
||||
fn record_failure(
|
||||
&self,
|
||||
id: i64,
|
||||
kind: Option<&str>,
|
||||
error: &str,
|
||||
next_attempt_at: i64,
|
||||
) -> Result<()> {
|
||||
let conn = self.conn.lock();
|
||||
if drop_replaced_rename(&conn, id)? {
|
||||
return Ok(());
|
||||
@@ -1284,8 +1310,8 @@ impl Db {
|
||||
"UPDATE pending_op
|
||||
SET attempts = attempts + 1, last_error = ?2, next_attempt_at = ?3,
|
||||
access_deferred_since = 0
|
||||
WHERE id = ?1",
|
||||
params![id, error, next_attempt_at],
|
||||
WHERE id = ?1 AND (?4 IS NULL OR kind = ?4)",
|
||||
params![id, error, next_attempt_at, kind],
|
||||
)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -3726,6 +3726,61 @@ fn a_create_withdrawn_after_it_failed_queues_a_trash_that_keeps_its_blob() {
|
||||
assert!(db.pending_ops().unwrap().is_empty());
|
||||
}
|
||||
|
||||
/// A create deleted after it was sent becomes a trash in its place. The
|
||||
/// create's backoff, and the failure of an attempt still on the wire, are the
|
||||
/// create's: the trash is due at once (`docs/BUGS.md` B171).
|
||||
#[test]
|
||||
fn a_create_deleted_after_it_was_sent_leaves_a_trash_that_is_due_at_once() {
|
||||
let db = Db::open_in_memory().unwrap();
|
||||
let parent = uid("parent").to_string();
|
||||
let create = |local: &str| {
|
||||
db.enqueue_op(&PendingOp {
|
||||
id: 0,
|
||||
kind: OP_CREATE.to_string(),
|
||||
uid: local.to_string(),
|
||||
parent_uid: Some(parent.clone()),
|
||||
name: Some(format!("{local}.txt")),
|
||||
blob_path: Some(format!("/staging/{local}")),
|
||||
meta_json: Some("{}".to_string()),
|
||||
created_at: 1,
|
||||
attempts: 0,
|
||||
last_error: None,
|
||||
next_attempt_at: 0,
|
||||
})
|
||||
.unwrap()
|
||||
.0
|
||||
};
|
||||
let failed = create("local~failed");
|
||||
db.record_attempt_failure(failed, OP_CREATE, "no answer", 50_000)
|
||||
.unwrap();
|
||||
let sending = create("local~sending");
|
||||
assert_eq!(
|
||||
db.claim_next_due_op(10).unwrap().map(|op| op.id),
|
||||
Some(sending)
|
||||
);
|
||||
|
||||
db.delete_ops_for_uid("local~failed").unwrap();
|
||||
db.delete_ops_for_uid("local~sending").unwrap();
|
||||
db.record_attempt_failure(sending, OP_CREATE, "No such file or directory", 50_000)
|
||||
.unwrap();
|
||||
|
||||
let ops = db.pending_ops().unwrap();
|
||||
assert_eq!(ops.len(), 2);
|
||||
for op in &ops {
|
||||
assert_eq!(op.kind, OP_TRASH);
|
||||
assert_eq!((op.attempts, op.last_error.as_deref()), (0, None), "{op:?}");
|
||||
assert_eq!(op.next_attempt_at, 0, "{op:?}");
|
||||
}
|
||||
db.record_attempt_failure(failed, OP_TRASH, "offline", 50_000)
|
||||
.unwrap();
|
||||
let trash = db
|
||||
.pending_ops()
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
.find(|op| op.id == failed);
|
||||
assert_eq!(trash.map(|op| op.attempts), Some(1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_landed_placeholder_never_overwrites_the_server_copy() {
|
||||
let db = Db::open_in_memory().unwrap();
|
||||
|
||||
@@ -394,8 +394,9 @@ impl Core {
|
||||
.min(DRAIN_BACKOFF_MAX)
|
||||
};
|
||||
warn!(uid = %op.uid, attempts, error = %err_str, "pending upload failed; will retry");
|
||||
if let Err(e) = self.db.record_op_failure(
|
||||
if let Err(e) = self.db.record_attempt_failure(
|
||||
op.id,
|
||||
&op.kind,
|
||||
&err_str,
|
||||
now_millis() + backoff.as_millis() as i64,
|
||||
) {
|
||||
|
||||
@@ -12,6 +12,31 @@ Conventions:
|
||||
|
||||
---
|
||||
|
||||
## B171 — A file deleted while its upload fails stays on Drive for the upload's backoff
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
**Found:** 2026-10-04, by the simulation suite (`one_client_on_a_good_link`, seed 9). The queue
|
||||
did not drain within 4 s of the settle: it still held a trash of `b.txt` with one attempt and
|
||||
the error "No such file or directory (os error 2)", due seconds later.
|
||||
|
||||
**Where:** `drop_doomed_ops` and `Db::record_op_failure` in `crates/pdfs-core/src/db/ops.rs`, and
|
||||
the drain loop in `crates/pdfs-fuse/src/drain.rs`.
|
||||
|
||||
**Cause.** A file deleted after its create was sent keeps its op as a trash, in case the create
|
||||
made the node and only its answer was lost (B151). The op kept the create's attempts, error and
|
||||
backoff. The drain also recorded the failure of a create attempt still on the wire on the row,
|
||||
which by then was the trash. Here the attempt failed because a write had replaced the blob it
|
||||
was reading, and the trash waited out the create's backoff.
|
||||
|
||||
**Fix.** The trash starts with no attempts and is due at once, as a create that becomes a
|
||||
revision already does. The drain records a failed attempt only if the op is still the kind it
|
||||
ran as (`Db::record_attempt_failure`).
|
||||
|
||||
**Test:** `a_create_deleted_after_it_was_sent_leaves_a_trash_that_is_due_at_once` in
|
||||
`crates/pdfs-core/src/db/tests.rs`.
|
||||
|
||||
---
|
||||
|
||||
## B170 — A write opened as an upload lands fails the next write with `EIO`
|
||||
|
||||
**Status:** Fixed (unverified).
|
||||
|
||||
@@ -70,6 +70,8 @@ by its local id, in the queue, the stored tree and pins alike.
|
||||
progress finish, and the rest waits for the next start (B169).
|
||||
- **Writing to a file again just as its upload finishes no longer fails.** A write right after
|
||||
it failed with "Input/output error" (B170).
|
||||
- **A file deleted while its upload was retrying is removed from Drive right away.** It no
|
||||
longer waits for the upload's next retry (B171).
|
||||
- **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).
|
||||
- **A change that waits for a name to be freed no longer waits seconds after it is.** A rename or
|
||||
|
||||
@@ -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: B127 and B141 to B170.
|
||||
- Fixes from the simulation and account runs since then: B127 and B141 to B171.
|
||||
- **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`.
|
||||
|
||||
@@ -112,7 +112,9 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase
|
||||
sent again, to the root (B168, fixed), and a stop that let a mirror pass go on
|
||||
until the runtime ended under it (B169, fixed). CI on 9bbc994 failed
|
||||
`one_client_on_a_good_link`: a write opened as an upload landed took the revision
|
||||
it replaced as its base, and the next write failed with `EIO` (B170, fixed).
|
||||
it replaced as its base, and the next write failed with `EIO` (B170, fixed). The
|
||||
suite's seed 9 found a deleted file's trash waiting out its create's backoff (B171,
|
||||
fixed).
|
||||
5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead.
|
||||
|
||||
## After 3.0.0
|
||||
|
||||
Reference in new issue
Block a user