[keyserver] Remove holders in upload deleters
ClosedPublic
Actions

Authored by bartek on Mon, Sep 30, 3:14 AM.

Details

Reviewers

tomek
ashoat

Commits

rCOMM4f2c8b6b4af4: [keyserver] Remove holders in upload deleters

Summary

Address ENG-9352.
Before calling DELETE FROM uploads, we need to select the extra column first and remove blob holders if present in the column JSON.

Depends on D13511

Test Plan

Used local Blob Service for testing.

For deleteUpload, used the delete_upload keyserver endpoint (easy way is canceling media message on web, with blob uploads enabled). Called the endpoint and verified that blob associated with the upload was deleted from DDB.

For deleting unassigned uploads: Manually modified MariaDB by setting containers to null and age to be older than 24h. Modified the cron job call to instead run during keyserver startup. Verified that holders were removed.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Mon, Sep 30, 3:14 AM

bartek held this revision as a draft.

bartek added a child revision: D13513: [keyserver] Remove holders in thread deleters.

Harbormaster completed remote builds in B31920: Diff 44665.Mon, Sep 30, 3:30 AM

bartek edited the test plan for this revision. (Show Details)Mon, Sep 30, 4:00 AM

bartek published this revision for review.Mon, Sep 30, 4:18 AM

bartek added inline comments.

keyserver/src/deleters/upload-deleters.js
76 ↗	(On Diff #44665)	Not sure how to handle failures for `removeBlobHolders` (HTTP call). Possible approaches: Ignore failures (`ignorePromiseRejections`) - might results in orphaned blobs Wait with DELETE for the holders request to succeed - the operation fails but they're still in DB so can be retried Store failed holders somewhere for later retry Besides, the `removeBlobHolders()` could potentially perform a retry-with-backoff

bartek mentioned this in D13513: [keyserver] Remove holders in thread deleters.Mon, Sep 30, 4:25 AM

bartek mentioned this in D13514: [keyserver] Remove holders in account deleters.Mon, Sep 30, 4:28 AM

ashoat added inline comments.Mon, Sep 30, 5:34 AM

keyserver/src/deleters/upload-deleters.js
76 ↗	(On Diff #44665)	My instinct is that this solution: Wait with DELETE for the holders request to succeed - the operation fails but they're still in DB so can be retried Is the easiest one here

ashoat accepted this revision.Mon, Sep 30, 5:36 AM

ashoat added inline comments.

keyserver/src/deleters/upload-deleters.js

76 ↗

(On Diff #44665)

Before landing, please update to:

const holdersQuery = SQL`
  SELECT extra
  FROM uploads
  WHERE container IS NULL
    AND user_container IS NULL
    AND creation_time < ${oldestUnassignedUploadToKeep}
`;
const [rows] = await dbQuery(holdersQuery);
const blobHolders = blobHoldersFromUploadRows(rows);
await removeBlobHolders(blobHolders);

const deletionQuery = SQL`
  DELETE u, i
  FROM uploads u
  LEFT JOIN ids i ON i.id = u.id
  WHERE u.container IS NULL
    AND u.user_container IS NULL
    AND creation_time < ${oldestUnassignedUploadToKeep}
`;
await dbQuery(deletionQuery);