Make gc() reachable so orphaned blobs and manifests are actually reclaimed
Maintainer thường phản hồi trong vòng 1 ngày
Đá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
- Khá 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/fs/gc.ts và packages/dofs/src/index.ts, sau đó kiểm tra entry point của Workspace trong packages/computer/src/workspace.ts và các gc test hiện có. Xác định liệu việc dọn dẹp là do caller điều khiển, dựa trên alarm hay opportunistic, expose entry point đã chọn và kết quả, đồng thời cập nhật bốn comment và các test để các row mồ côi được thu hồi trong khi các blob được liên kết vẫn không bị thay đổi.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Mô tả
Summary
packages/dofs/src/fs/gc.ts implements orphan reclamation for vfs_blobs and vfs_manifests. It is transactional, it has a one-hour safety window, it has unit tests, and the schema carries a partial index added specifically to keep its manifest sweep from being quadratic. It is not exported and nothing calls it, so no orphan is ever reclaimed.
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
Verified against main at 76d9e75:
gcis not exported frompackages/dofs/src/index.ts. That file exports 30-plus symbols including every sync building block (applyChanges,coalesceChanges,fetchChanges,stageBlob,writeWatermark,buildManifest).gcis absent.packages/dofs/package.jsonexposes only"."and"./testing", so there is no deep import path either.- Searching for
gc(acrosspackages, excludingfs/gc.tsand tests, returns four hits, and all four are comments.
Those four comments are the reason this matters. Three of them are load-bearing justifications for leaving garbage behind:
packages/dofs/src/fs/writeFile.ts:138- "Failure mid-stream leaves blob rows behind;gc()reaps orphans on a later pass."packages/dofs/src/fs/writeFile.ts:152- "orphan blob rows thatgc()then has to reap."packages/dofs/src/fs/writeFile.ts:961- "are cleaned up by a latergc()pass."packages/computer/src/mounts/types.ts:26- "rows may briefly linger and are reaped bygc()."
There is no later pass. Every interrupted or failed streaming write leaks blob rows, and the bytes go with them through the vfs_blob_bytes foreign key. In a Durable Object, where SQLite is the durable substrate and storage is finite, a cleanup that is documented but never runs is a slow leak with no operator remedy short of recreating the workspace.
The surrounding evidence suggests this is an oversight rather than a decision:
gc.ts:20-53runs both deletes in onedb.transactionSync, withDEFAULT_SAFETY_WINDOW_MSof one hour and the comment that the generous default exists so "a misconfigured GC pass cannot wipe blobs the application is actively writing." That is operational thinking, not dead code.packages/dofs/src/schema/core.ts:51-57adds the partial indexvfs_nodes_by_manifest_hashwith the comment: "gc/manifests checks every manifest row against vfs_nodes via a correlated NOT EXISTS [...] Without this index gc full-scans vfs_nodes per candidate manifest - O(N x M)." Schema work was done for a function that cannot run.packages/dofs/src/sync/blobs.ts:10hasstageBlobtouchlast_seen"so the bytes don't get reaped by an interleaved gc," so the sync path already coordinates with it.packages/dofs/README.mdnotes thesrc/fs/*primitives,gcamong them, "are not re-exported from the package root yet."
Goals
-
Make
gcreachable: exportgc,GcOptions, andGcResultfrom the@cloudflare/dofspackage root, alongside the sync building blocks already exported there. No behaviour change. -
Give it a host-side entry point on
Workspacethat runs the sweep and returns{ blobsFreed, manifestsFreed }, so a consumer can reclaim on its own schedule and observe what was freed. -
Decide who triggers it. This is the part I would rather ask than assume, and it is the part that actually closes the leak. Three options with different cost profiles:
- Caller-driven only. Smallest change, no policy baked in, but the leak persists for every consumer who does not know to call it.
- A Durable Object alarm. The natural home for periodic maintenance, but
packages/computer/src/workspace.ts:57notes a backend "cannot own a Durable Object alarm. Each backend has at most one intent," and the container keep-alive already uses alarms, so the alarm is contended. - Opportunistic, after a sync tick or the post-exec pull, guarded by the existing one-hour
last_seenwindow plus a cheap "anything deleted since last sweep" check so an idle workspace does no work. Cheapest to reason about, but adds work to a latency-sensitive path.
The safety window means correctness does not depend much on the choice; cost and latency do.
-
Make the four comments true. If the answer is caller-driven only, they should be amended to say the caller is responsible, so the code stops asserting a cleanup the library does not perform.
Out of scope: no change to gc's predicates, safety window, or transaction shape. It looks correct as written; it is only unreachable. Tombstone pruning in vfs_changes is a separate gap I am filing alongside this one.
Deleting gc and its index instead is a coherent alternative if orphans are considered acceptable, and worth naming so the decision is explicit. It would mean rewriting the four comments and accepting the leak from interrupted writes.
Example
// packages/dofs/src/index.ts - currently absent
export { gc, type GcOptions, type GcResult } from "./fs/gc.js";
// host-side entry point
const { blobsFreed, manifestsFreed } = await workspace.gc();
For tests, the now injection already in GcOptions exists so the clock can be pinned, so no new test infrastructure is needed. The coverage worth adding is the reachability regression that would have caught the current state (importing gc from the package root and running it), plus driving an interrupted streaming write through the writeFile.ts:138 path, asserting orphan rows exist, sweeping past the safety window, and asserting they are gone while linked blobs are untouched.
Happy to open a PR for the export and the entry point if that direction works, and to hold the trigger question until you have picked one.
- Ngôn ngữ chính
- TypeScript
- Star
- 9.5k
- Fork
- 552
- Merge trung bình
- 2 ngày 4 giờ
- Pull request đã merge (30 ngày)
- 49
Chuẩn bị môi trường
- Không có Dockerfile hay tệp Docker Compose
- Có mẫu pull request
- Đọc hướng dẫn đóng góp
Bắt đầu từ đâu
- Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
- 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.
- Fork repository và làm thay đổi trên một nhánh.
- Mở pull request có tham chiếu số hiệu của issue.
Issue khác của cloudflare/computer
-
enhancement
Độ khó 4/5 3-5 ngày Mức phù hợp với người mới 30/100
cloudflare/computer#224 · 5 bình luận · 1 reaction ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Always run the "next" computerd workflow on push to the `release` branchCó thể đã có người làm @aron-cf đã nhận 1 ngày trước. Đang mởbug
cloudflare/computer#222 · 2 bình luận · 1 reaction · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Make just-bash and acorn optional peer dependencies, pulled in only by the backend that needs themĐang mởenhancement
Độ khó 3/5 1-2 ngày Mức phù hợp với người mới 45/100
cloudflare/computer#220 · 3 bình luận · 1 reaction ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Workspace namespaces: more than one Workspace per Durable Object storageCó thể đã có người làm @aron-cf đã nhận 1 ngày trước. Đang mởenhancement
cloudflare/computer#219 · 1 bình luận · 1 reaction · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
-
git: host-held credentials for model-driven clone, fetch, pull and pushCó thể đã có người làm @aron-cf đã nhận 1 ngày trước. Đang mởenhancement
cloudflare/computer#218 · 3 bình luận · 1 reaction · 1 người được giao ·
Maintainer thường phản hồi trong vòng 1 ngày
Tất cả issue của cloudflare/computer
Issue tương tự
-
bug confirmed perf
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 72/100
videojs/video.js#9400 · 1 bình luận ·
Maintainer thường phản hồi trong vòng 1 ngày
-
good first issue hacktoberfest
Độ khó 2/5 Nửa ngày Mức phù hợp với người mới 70/100
HelpCode-ai/anythingmcp#996 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
Add: CanalPlusActionEurope.nlĐang mởcheck:passed streams:add
Độ khó 1/5 1-3 giờ Mức phù hợp với người mới 72/100
Maintainer thường phản hồi trong vòng 2 ngày
-
beta technical-medium ui
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 62/100
walletbeat/walletbeat#1625 ·
Maintainer thường phản hồi trong vòng 1 ngày
-
[Good First Issue]: Add unit tests for NetworkVersionInfoCó thể đã có người làm @attilayener đã nhận hôm nay. Đang mởGood First Issue hacktoberfest
Độ khó 2/5 1-3 giờ Mức phù hợp với người mới 85/100
hiero-ledger/hiero-sdk-js#4489 ·
Maintainer thường phản hồi trong vòng 1 ngày