We run a multi-tenant deployment where deleting a document has to be verifiable — a data-retention requirement means we need to be able to show the bytes are gone, not just that we asked for them to go. Working through 0.3.1 I don't think that's currently possible from outside yhub, and I'd like to check whether you'd want the missing pieces upstream.
What blocks it
1. Enumeration can lose rows. retrieveDoc(room, { references: true }) only reports a reference once the plugin retrieve for it succeeded (retrieved && references?.push(...), src/persistence.js). A temporarily unreadable object hides its row, and with it the assetId needed to ever delete that object — so a transient storage error strands a row permanently, invisible to both deleteReferences and any check based on references.
2. Rows go before objects. deleteReferences fires the plugin deletes without awaiting them and awaits the DELETE. A failed object delete therefore removes the only rows naming the object.
3. Plugin delete returns before deleting. setTimeout(..., 10_000) + return true, errors swallowed. Promise<boolean> reads as "deleted"; it means "scheduled".
To be clear, I don't think (2) and (3) are bugs — they're correct for compaction, which is what calls them. Deferring protects readers still on the previous t, awaiting object deletes would put storage latency on every compaction, and keeping rows on failure would leak them on the hot path. The gap is that there's no second mode for the case where the deletion has to be provable.
This matters more than it might look because store() offloads unconditionally for branch === 'main' (no size threshold, src/plugins/s3.js), so effectively every byte lives in object storage and the row is a stub. Deleting rows deletes pointers.
Also: deleted documents come back
Worth flagging separately, since it caught us out. store() is an insert with no existence check — the rows are the document — so anything that writes after a delete silently recreates it, and compaction merges the whole document rather than just new edits, so one leftover compact task restores everything.
recheckAuth(room, { forceDisconnect: true }) in 0.3.1 handles the connected-client half nicely (thank you — that landed right as we were looking for it). What it doesn't cover is a compaction task that's already queued, or unsafePersistDoc, which bypasses the stream and the compaction-disabled set entirely. Our plan is disableCompaction + quarantine before deleting, which is close but leaves unsafePersistDoc open and accumulates entries keyed to documents that no longer exist.
Is a deleted/stale marker consulted by store(), the compact-task enqueue, and unsafePersistDoc something you'd consider? That would make the whole thing airtight rather than close, and verification meaningful — you'd be checking a closed system.
Proposed
#57 adds the three self-contained pieces, additive, existing behaviour untouched:
persistence.listRoomAssets(room) — complete enumeration, no plugin callspersistence.deleteReferencesNow(references) — objects first and awaited, rows only once confirmedPersistencePlugin.deleteNow — optional immediate, awaited delete (S3 implemented)
If you'd rather expose this as a single deleteDoc(room) that handles all branches, disconnects writers, and verifies, those three are its ingredients and I'm happy to reshape the PR that way.
We can also hand-roll all of it against persistence.sql, decodeAny, and the assetIdToString path convention — but that would freeze the table schema, asset encoding, and object layout into our deletion path and break silently whenever any of them change. That's the reason I'd rather it live upstream than in our integration.
Environment: @y/hub 0.3.1, S3 plugin (plus our own Azure Blob and GCS plugins, which follow the S3 plugin's deferred-delete shape and would need matching deleteNow).
We run a multi-tenant deployment where deleting a document has to be verifiable — a data-retention requirement means we need to be able to show the bytes are gone, not just that we asked for them to go. Working through 0.3.1 I don't think that's currently possible from outside yhub, and I'd like to check whether you'd want the missing pieces upstream.
What blocks it
1. Enumeration can lose rows.
retrieveDoc(room, { references: true })only reports a reference once the plugin retrieve for it succeeded (retrieved && references?.push(...),src/persistence.js). A temporarily unreadable object hides its row, and with it theassetIdneeded to ever delete that object — so a transient storage error strands a row permanently, invisible to bothdeleteReferencesand any check based on references.2. Rows go before objects.
deleteReferencesfires the plugin deletes without awaiting them and awaits theDELETE. A failed object delete therefore removes the only rows naming the object.3. Plugin
deletereturns before deleting.setTimeout(..., 10_000)+return true, errors swallowed.Promise<boolean>reads as "deleted"; it means "scheduled".To be clear, I don't think (2) and (3) are bugs — they're correct for compaction, which is what calls them. Deferring protects readers still on the previous
t, awaiting object deletes would put storage latency on every compaction, and keeping rows on failure would leak them on the hot path. The gap is that there's no second mode for the case where the deletion has to be provable.This matters more than it might look because
store()offloads unconditionally forbranch === 'main'(no size threshold,src/plugins/s3.js), so effectively every byte lives in object storage and the row is a stub. Deleting rows deletes pointers.Also: deleted documents come back
Worth flagging separately, since it caught us out.
store()is an insert with no existence check — the rows are the document — so anything that writes after a delete silently recreates it, and compaction merges the whole document rather than just new edits, so one leftover compact task restores everything.recheckAuth(room, { forceDisconnect: true })in 0.3.1 handles the connected-client half nicely (thank you — that landed right as we were looking for it). What it doesn't cover is a compaction task that's already queued, orunsafePersistDoc, which bypasses the stream and the compaction-disabled set entirely. Our plan isdisableCompaction+quarantinebefore deleting, which is close but leavesunsafePersistDocopen and accumulates entries keyed to documents that no longer exist.Is a deleted/stale marker consulted by
store(), the compact-task enqueue, andunsafePersistDocsomething you'd consider? That would make the whole thing airtight rather than close, and verification meaningful — you'd be checking a closed system.Proposed
#57 adds the three self-contained pieces, additive, existing behaviour untouched:
persistence.listRoomAssets(room)— complete enumeration, no plugin callspersistence.deleteReferencesNow(references)— objects first and awaited, rows only once confirmedPersistencePlugin.deleteNow— optional immediate, awaited delete (S3 implemented)If you'd rather expose this as a single
deleteDoc(room)that handles all branches, disconnects writers, and verifies, those three are its ingredients and I'm happy to reshape the PR that way.We can also hand-roll all of it against
persistence.sql,decodeAny, and theassetIdToStringpath convention — but that would freeze the table schema, asset encoding, and object layout into our deletion path and break silently whenever any of them change. That's the reason I'd rather it live upstream than in our integration.Environment:
@y/hub0.3.1, S3 plugin (plus our own Azure Blob and GCS plugins, which follow the S3 plugin's deferred-delete shape and would need matchingdeleteNow).