Conversation
lookup/readdir/readdir_plus awaited pull_documents inline. network latency inside a kernel-bounded syscall. mkdir bursts (webpull) hit the soft-mount timeout and returned ETIMEDOUT. new HydrationScheduler enqueues misses and drains them with a bounded worker pool, dedup by key, short negative-TTL. fix the dirty_since guard on the attach-by-path reconcile branch first so async hydration can't clobber pending local writes. verified parity (real fs vs smfs) on macos/nfs and linux/fuse: 120-page webpull, identical tree + sha256, 0 errors.
![vorflux[bot]](https://avatars.githubusercontent.com/in/2483280?s=60&v=4){kind=small}
| if dirty_since > ms { | ||
| self.db.set_remote_id(ino, &doc.id); | ||
| self.db.push_queue_set_remote_id(filepath, &doc.id); | ||
| return Ok(ReconcileOutcome::SkippedDirty); |
There was a problem hiding this comment.
This push_queue_set_remote_id call takes filepath (a &str derived from doc.filepath), but push_queue_set_remote_id in db.rs matches on the filepath key already in the push queue. If the file was locally created before the remote doc existed, the push queue entry filepath might differ from doc.filepath. Worth verifying this matches the key used in push_queue_upsert when the file was originally enqueued -- otherwise this UPDATE matches zero rows.
|
|
||
| Ok(Some(Vec::new())) | ||
| let mut names: Vec<String> = Vec::new(); | ||
| if ino == ROOT_INO && self.api.is_some() && !names.contains(&PROFILE_NAME.to_string()) { |
There was a problem hiding this comment.
Minor: !names.contains(&PROFILE_NAME.to_string()) is always true here since names was just created as Vec::new() on line 1186. Dead code in this branch -- could simplify to just if ino == ROOT_INO && self.api.is_some().
| let mut inner = self.inner.lock(); | ||
| inner | ||
| .recent | ||
| .retain(|_, ts| now.duration_since(*ts) < self.negative_ttl); |
There was a problem hiding this comment.
retain scans the entire recent HashMap on every enqueue. With 2s TTL and 4 workers, n stays small in practice. But consider amortizing (e.g., only retain every Nth call or when recent.len() > threshold) if burst sizes grow.
|
|
||
| // Attach remote_id even on skip so the queued push promotes | ||
| // Create→Update instead of POSTing a duplicate. | ||
| if let Some(dirty_since) = self.db.get_dirty_since(ino) { |
There was a problem hiding this comment.
Subtle ordering concern: if updated_ms is None (parse failure on doc.updated_at) and dirty_since is Some, the if let Some(ms) = updated_ms check fails, we fall through to line 359 and unconditionally set remote_id + rewrite content -- potentially clobbering a local dirty write. Consider: if dirty_since.is_some() && updated_ms.is_none(), return SkippedDirty as a safety net.
|
@Prasanna721 - The reviews from Vorflux look legit |
2 participants
vfs lookup/readdir/readdir_plus awaited pull_documents inline,
putting network latency inside the kernel-bounded syscall path.
under a burst of 120 concurrent mkdirs (webpull's normal behavior)
macos NFS would soft-timeout and the kernel returned ETIMEDOUT.
fix is structural: the syscall must be local-only.
added HydrationScheduler (in-memory, singleflight + bounded
workers + short negative-TTL) and swapped the three hot-path call
sites to enqueue instead of await. delta pull, push queue, smfs
sync are unchanged.
also ships the dirty_since guard on the attach-by-path reconcile
branch. prerequisite: without it, async hydration could clobber a
local write still pending in the push queue.
how I tested:
mount, both macos/NFS and linux/FUSE: identical tree and identical
sha256 across all 120 files, 0 mkdir errors, smfs run finished
faster than the real fs run