Design doc for Upload/AssetBlob garbage collection #2068
Conversation
mvandenburgh
force-pushed
the
asset-blob-upload-gc-doc
branch
2 times, most recently
from
98572f7
to
bd9740f
Compare
mvandenburgh
force-pushed
the
asset-blob-upload-gc-doc
branch
from
bd9740f
to
d2f3123
Compare
| @@ -0,0 +1,43 @@ | |||
| # Upload and Asset Blob Garbage Collection | |||
|
|
|||
| This document presents a design for garbage collection of uploads and asset blobs in the context of the S3 trailing delete feature. It explains the need for garbage collection and describes the scenarios of orphaned uploads and orphaned asset blobs. The implementation involves introducing a new daily task to query and delete uploads and asset blobs that meet certain criteria. The document also mentions the recoverability of uploaded data and provides a GitHub branch for the implementation. | |||
There was a problem hiding this comment.
Please add the GitHub branch to this doc.
There was a problem hiding this comment.
Thank you, @mvandenburgh. I am new to this garbage collection effort, but this design doc looks good from a high-level perspective.
|
Hi @yarikoptic, when you have a chance, can you please review this design doc? Thank you. |
|
|
||
| ### Orphaned AssetBlobs | ||
|
|
||
| In this case, assume that the user properly completes the multipart upload flow and "finalizes" the `Upload` record such that it is now an `AssetBlob`, but they do not send a request to associate the new blob with an `Asset`. That `AssetBlob` record and associated S3 object will remain in the database/bucket indefinitely. |
There was a problem hiding this comment.
Or if Asset was removed somehow, e.g. via future implementation of Assets GC.
There was a problem hiding this comment.
Updated wording to include this 959e9b4
|
|
||
| Due to the trailing delete lifecycle rule, the actual uploaded data will remain recoverable for up to 30 days after this deletion, after which the lifecycle rule will clear it out of the bucket permanently. | ||
|
|
||
| In order to facilitate restoration of deleted data, as well as for general auditability of the garbage collection feature, a new database table will be created to store information on garbage-collection events. Rows in this new table will be garbage-collected themselves every 30 days, since that is the number of days that the trailing delete feature waits before deleting expired object versions. In other words, once the blob is no longer recoverable via trailing delete in S3, the corresponding `GarbageCollectionEvent` should be deleted as well. |
There was a problem hiding this comment.
why not to include this information in the Audit table?
- It might be trickier to collate all information on activities of a user if we would need to harvest it across different tables
- e.g. to collect information on stats of per-user uploads
- Eventually I hope we would get "legit" API endpoint for canceling/removing uploads. Such events should also go into Audit. So why not to place GC'ed also there?
There was a problem hiding this comment.
@waxlamp and I discussed this as a possibility when I was writing this doc, and came to the conclusion that audit events are really intended to represent "user-driven" actions, while garbage collection is a "system-driven" action. In other words, an event in the audit table should be associated with a specific user. Garbage collection is an automated process that doesn't fit into that category.
If someday we decided to expose data restoration as a user-facing feature via the API, that would be a different matter and would likely go into the audit table. I expect cancelling uploads via the API would also go into the audit table, as that would be user-driven as well.
| We will introduce a new celery-beat task that runs daily. This task will | ||
|
|
||
| - Query for and delete any uploads that are older than the multipart upload presigned URL expiration time (this is currently 7 days). | ||
| - Query for and delete any AssetBlobs that are (1) not associated with any Assets, and (2) older than 7 days. |
There was a problem hiding this comment.
There is a possibility for a race condition that user is uploading a known to archive blob which is about to be GCed, and would initiate a new Asset by passing AssetBlob id right when it would be GC'ed.
Very unlikely, but can happen. Then we might end up with an error during new Asset creation and somehow inform client that it needs to re-do the logic for upload.
I do not know if we should somehow provision to avoid such a possible race since I do not see non-complicated solution. But may be implementation could really minimize the duration from "query" to "delete" as to not delay DELETE for considerable amount of time upon query.
There was a problem hiding this comment.
I think in that case, the server should return a 400 error when attempting to create the Asset, and it would be up to the client to infer that the AssetBlob doesn't exist anymore. Since creating an asset requires multiple API requests, I don't see any way to guard against this. One could make the argument that once an API response comes back, the info is immediately stale and potentially out of date.
Co-authored-by: Yaroslav Halchenko <[email protected]>
This design document proposes a design for garbage collection of
UploadandAssetBlobrecords. A prerequisite to enabling this was the "trailing delete" feature that has landed in production (see doc)A design document for garbage collection of
Assetrecords will follow this one.@yarikoptic @satra PTAL and comment here if you have any objections or general thoughts about the proposed process.