Skip to content

Commit f5230d1

Browse files
pmaxhoganclaude
andauthored
fix(core): commit file_state for a create that skipped post-upload so the next scan updates instead of re-creating (#146)
## Mechanism (#144) When a plain first-CREATE upload's local file changes during the hash->upload->post-check window, the executor returned `SkipPostUpload` and kept the pending op with **no** `file_state` row, relying on the startup-gated reconcile pass to adopt the just-created object by op-uuid. Mid-run nothing adopts it, so the next scan saw a row-less path and planned a **second** create - duplicating live objects until the next app restart. The chaos `frequent-edits` gate saw ~20 duplicate `hot.txt` objects under a 3ms injected store delay (found during #141's CI run; pre-existing in v2.0.1, not introduced by #141). ## Fix - symmetry with reconcile's adopt-requeue arm `upload_and_commit`'s post-upload identity recheck now routes through `settle_post_upload_change`. For a **plain create** (no pre-existing row, not a versioned change) it durably commits the *same* force-rescan `file_state` row that reconcile's `adopt_reconciled` requeue arm would eventually write: - `drive_file_id` = the just-created object, - `mtime_ns = REQUEUE_FORCE_RESCAN_MTIME_NS` (`i64::MIN`) so the next FastPath scan always re-emits the path, - the uploaded (now-stale) blake3, the on-Drive md5/size, status `Pending`, via `commit_create_result`, which upserts the row **and** deletes the op in one transaction. The next scan re-uploads as an **UPDATE** against the same object: exactly one live object per path, no restart required. ### Design note The issue sketched "thread the created id through the error path". The id is already in scope at the detection point, so the helper commits in place and returns `Ok(Skipped)` - a smaller seam into the hardened `UploadError` enum with an identical durable outcome. ## Invariants preserved - **Atomic durable commit**: row upsert + op delete are one transaction (`commit_create_result`) - no split write. - **Crash-safety fallback**: a crash *before* the commit leaves the op (with its op-uuid) and no row, so the pre-#144 orphan+reconcile adoption still runs on the next boot (covered by `crash_mid_upload_adopts_orphan_without_duplicate`). - **Reconcile idempotency**: the commit deletes the op atomically with the row, so reconcile never finds this op again - no double-adopt (verified vacuous + covered by a restart test). - **UPDATE and versioned-create** keep the pre-#144 `SkipPostUpload` path (their existing row already points at an object; the next scan updates/versions against it, never re-creating the path). - **Encryption**: the created id + ciphertext `encrypted_remote_path` flow into the row identically to a plaintext source. - **DeferToReconcile** (ambiguous create) is untouched - it has no known id to point at, so it stays reconcile-driven (comment updated to explain the distinction). ## Tests - `create_changed_after_upload_commits_force_rescan_row_no_duplicate` - deterministic race, then the NEXT scan in the same process UPDATEs the same id (exactly one live object). - `create_skip_post_upload_row_survives_reconcile_no_double_adopt` - restart mid-window: committed row + reconcile pass stays one object. - `update_changed_after_upload_keeps_existing_id_no_duplicate` - update carve-out unchanged. - `create_changed_after_upload_encrypted_commits_force_rescan_row` - encrypted path. - `frequent_edits_slow_store_holds_one_object_144` - chaos frequent-edits with a 3ms injected store delay holds exactly one object. Full `driven-core` lib suite (336) + `e2e_fake` acceptance suite (24) + clippy green. Closes #144 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01QZQVP2tUuTLh8oL31D8heC --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 62d543a commit f5230d1

2 files changed

Lines changed: 551 additions & 67 deletions

File tree

crates/driven-chaos/src/scenarios/mutation.rs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,6 +1716,71 @@ mod tests {
17161716
);
17171717
}
17181718

1719+
/// Issue #144 regression: `frequent-edits` racing a SLOW store (a 3ms
1720+
/// per-request delay, below the 4ms mutation cadence) must still converge to
1721+
/// EXACTLY ONE live object. The slow store widens the very first upload's
1722+
/// hash->upload->post-check window so the mutator reliably lands a change
1723+
/// mid-window: the first upload of `hot.txt` is a plain CREATE, so that
1724+
/// mutation trips create-`SkipPostUpload`. Pre-#144 the just-created object
1725+
/// was left a row-less orphan and the next scan re-created it (the #141
1726+
/// diagnosis saw ~20 duplicate `hot.txt` objects under this exact delay);
1727+
/// the fix commits a force-rescan row so the next scan UPDATES the same
1728+
/// object instead. The assertion holds whether or not the race trips on a
1729+
/// given machine (so it is not flaky), and went RED pre-fix whenever it did.
1730+
#[tokio::test]
1731+
async fn frequent_edits_slow_store_holds_one_object_144() {
1732+
let remote =
1733+
Arc::new(InMemoryRemoteStore::new().with_slow_responses(Duration::from_millis(3)));
1734+
let h = SoakHarness::boot(remote).await.unwrap();
1735+
write_file(&h.src_root, "hot.txt", b"v0").unwrap();
1736+
1737+
// Rewrite hot.txt with a size-varying body every 4ms (> the 3ms store
1738+
// delay) so the scanner's (size, mtime) fast-path sees every edit and a
1739+
// mutation reliably lands inside the widened upload window.
1740+
let hot = h.src_root.join("hot.txt");
1741+
let counter = Arc::new(AtomicU64::new(1));
1742+
let counter_t = counter.clone();
1743+
let mutator = MutatorThread::spawn(MUTATE_EVERY, move || {
1744+
let n = counter_t.fetch_add(1, Ordering::Relaxed);
1745+
let body = format!("edit-{n}-{}", "x".repeat((n % 37) as usize));
1746+
std::fs::write(&hot, body.as_bytes()).is_ok()
1747+
});
1748+
1749+
for _ in 0..SOAK_ITERATIONS {
1750+
run_cycle_capture(h.orch()).await.unwrap();
1751+
}
1752+
mutator.stop_and_join();
1753+
1754+
// Settle on a final body and drain the pipeline.
1755+
let final_body = b"final-frequent-edits-slow-store";
1756+
std::fs::write(h.src_root.join("hot.txt"), final_body).unwrap();
1757+
drain_to_steady_state(&h).await.unwrap();
1758+
1759+
// The #144 invariant: exactly one live object, holding the final bytes.
1760+
let count = h.live_object_count().await.unwrap();
1761+
assert_eq!(
1762+
count, 1,
1763+
"slow-store frequent-edits must converge to exactly 1 live object (no #144 duplicate), found {count}"
1764+
);
1765+
let children = h
1766+
.remote
1767+
.list_folder(
1768+
&h.folder,
1769+
&driven_drive::remote_store::DriveContext::MyDrive,
1770+
)
1771+
.await
1772+
.unwrap();
1773+
let live = children
1774+
.iter()
1775+
.find(|e| !e.trashed)
1776+
.expect("the one object");
1777+
let remote_bytes = download_bytes(&h.remote, &live.id).await.unwrap();
1778+
assert_eq!(
1779+
remote_bytes, final_body,
1780+
"the single object must hold the final local edit"
1781+
);
1782+
}
1783+
17191784
/// The lock rows gate on Windows; the rest run anywhere.
17201785
#[test]
17211786
fn lock_rows_require_windows() {

0 commit comments

Comments
 (0)