Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

Prune acknowledged tombstones from vfs_changes so the table stops growing without bound

Đang mở
#67 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@Hotragn đang làm issue này rồi.

Từ ngày 13/8/2026.

  • #1 của @Hotragn — đang mở

Đánh giá

Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức phù hợp với người mới
45/100
Loại issue
Tính năng
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Ít trao đổi
Công nghệ
sqlite, typescript

Hướng nghiên cứu

Bắt đầu với packages/dofs/src/sync/watermarks.ts, packages/dofs/src/sync/coalesce.ts và các đường dẫn đồng bộ hóa RPC được tham chiếu để lần theo cách xử lý watermark và cursor. Chạy các case hiện có trong watermarks.test.ts và apply.test.ts bằng SQLiteTestStorage. Hoàn tất có nghĩa là pruning mang tính giao dịch và fail-closed trên mọi backend, các fetch cursor cũ sẽ kích hoạt đường dẫn truncation có kiểu hiện có, và test multi-backend mới chạy thành công mà không thay đổi schema hoặc wire format.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

Summary

vfs_changes is append-only. Nothing in the repository ever deletes from it, so every rm a workspace performs is retained for the lifetime of the Durable Object. docs/03_filesystem_schema.md already flags this as planned-but-unwired. This proposes wiring it, with a safer predicate than the one the doc sketches.

Filing as an issue because CONTRIBUTING.md routes feature requests to Discussions and Discussions are not enabled on this repo, so the documented link 404s (#53). Happy to move this to a Discussion if that gets turned on.

Background and motivation

docs/03_filesystem_schema.md:197 states the intent and the current state:

Pruning (planned; not yet wired). The target behaviour is to delete rows with rev <= pushRev in the same transaction that advances pushRev [...] Today writeWatermark only updates _vfs_watermark; there is no DELETE FROM vfs_changes anywhere in the package, so the table grows unboundedly with delete activity. Cheap to add once the apply path becomes push-atomic.

Both halves verify against main at 76d9e75:

  • Searching vfs_changes across packages/dofs/src and packages/computer/src returns inserts (sync/changes.ts:10, fs/rename.ts:225) and selects (sync/coalesce.ts:86,90, sync/changes.ts:98). There is no DELETE.
  • writeWatermark (packages/dofs/src/sync/watermarks.ts:85-92) is a single writeWatermarkValue call, no transaction, no prune.

There is a second consequence beyond storage size. packages/dofs/src/sync/changes.ts:96 justifies a per-path lookup with an invariant that does not currently hold:

an indexed scan by path is cheap because vfs_changes is bounded by the watermark window.

Nothing bounds it. That lookup runs on every materialiseChange, and the (path, id DESC) index at packages/dofs/src/schema/sync.ts:25 grows with cumulative historical delete count rather than with live state. An agent loop that repeatedly builds and cleans a tree (npm install, rm -rf node_modules) adds one row per removed path per cycle, permanently. fs/rename.ts:225 inserts one tombstone per path in a moved subtree via a CTE, so a single recursive delete or directory rename is O(subtree) rows.

Why the doc's one-line predicate is not safe as written

rev <= pushRev is correct for one backend with one consumer. Neither assumption holds today.

Multiple backends. _vfs_watermark is keyed PRIMARY KEY (k, backend) (packages/dofs/src/schema/sync.ts:31-36), and every watermark accessor takes a backend parameter. The README says a Workspace may register multiple backends under stable IDs. Pruning at backend A's pushRev destroys tombstones backend B has not been told about, and B never learns those paths were deleted.

Served fetches use a cursor that is not ours. coalesceChanges has two consumers:

  • packages/rpc/src/sync-driver.ts:300 reads from our own sincePush watermark.
  • packages/rpc/src/server.ts:189 serves fetchChanges({ after }), where after is the peer's cursor arriving over the wire. It is not compared against any local watermark before the tombstone query at coalesce.ts:86 runs.

So a peer resuming from a cursor below the prune point silently receives no tombstones for the pruned range, keeps files the DO considers deleted, and nothing reports it. Silent divergence is a worse failure than unbounded growth.

Goals

  • Bound vfs_changes so it tracks live delete activity rather than cumulative history, making the changes.ts:96 invariant true rather than aspirational.
  • Prune only at a point no consumer can still need. Concretely, a floor of MIN(v) across _vfs_watermark for both pushRev and fetchRev, across every backend, with an unknown or never-written backend contributing 0 and stopping the prune entirely.
  • Perform the delete in the same transaction that advances the watermark, which is what the doc asks for. writeFetchCursor (watermarks.ts:105-114) already demonstrates that exact db.transactionSync shape eight lines below writeWatermark.
  • Make an under-served fetch loud instead of silent. If a peer's after.rev is below the floor, raise a typed truncation error so the client routes into the existing rev-0 re-baseline in reconcileWatermarks (sync-driver.ts:396-435) rather than quietly missing deletions. ELOG_TRUNCATED is the existing precedent for exactly this trade-off on the exec log, and assertAppliedPushCursor is the house style for failing loudly on a cross-side invariant.
  • No schema migration and no wire-format change. Tables and indexes are unchanged; only rows are removed.

Out of scope: blob and manifest reclamation, which is a separate gap I am filing alongside this one.

Example

The shape of the change in packages/dofs/src/sync/watermarks.ts:

// New: the lowest rev any consumer could still resume from.
// Fail closed - a backend with no watermark row contributes 0.
export function prunableRev(db: Database): number { /* MIN over pushRev + fetchRev, all backends */ }

export function writeWatermark(db, key, value, backend = DEFAULT_BACKEND_ID): void {
  db.transactionSync(() => {
    writeWatermarkValue(db, key, value, backend);
    db.run("DELETE FROM vfs_changes WHERE rev <= ?", prunableRev(db));
  });
}

The test that distinguishes this from the doc's literal predicate: register backends A and B, advance only A, and assert no rows are pruned.

The tests would live in packages/dofs/src/sync/watermarks.test.ts and apply.test.ts, which already build real databases via SQLiteTestStorage and already call writeWatermark directly (apply.test.ts:555-556,616-618,703-712).

If it helps review, the truncation error (the fourth goal) is a correctness improvement that stands on its own and could land first as a smaller change, since a peer resuming from a stale cursor is already reachable today through reconcileWatermarks resets.

Happy to open a PR if the direction works for you, or to adjust the predicate if there is a resumable cursor I have missed.

Ngôn ngữ chính
TypeScript
Star
9.6k
Fork
558
Merge trung bình
1 ngày 18 giờ
Pull request đã merge (30 ngày)
52

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của cloudflare/computer

Tất cả issue của cloudflare/computer

Issue tương tự

Thêm issue về TypeScript

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.