Page MenuHomePhabricator
Differential D9355

[blob] Delete S3 objects during cleanup
ClosedPublic
Actions

Authored by bartek on Oct 4 2023, 4:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 17, 12:17 AM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:55 PM
Unknown Object (File)
Tue, Mar 4, 8:49 PM
View All Files
Subscribers
ashoat
tomek
will

Details

Reviewers
michal
varun
• jon
Commits
rCOMM73401a024533: [blob] Delete S3 objects during cleanup
Summary

Final part of blob cleanup logic - added deletion of S3 objects.

Depends on D9352.

Test Plan

Locally, uploaded a few items to S3 with object names hashA, hashB.
Created blob item rows in DDB: one with 'unchecked' attribute (hashA) and the other without (hashB).
No holders were given.
Ran the cleanup: only hashA file was deleted from S3.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Oct 4 2023, 4:10 AM
bartek held this revision as a draft.
Herald added a reviewer: • jon. · View Herald TranscriptOct 4 2023, 4:10 AM
Herald added subscribers: will, tomek, ashoat. · View Herald Transcript
Harbormaster completed remote builds in B22964: Diff 31628.Oct 4 2023, 4:28 AM
bartek published this revision for review.Oct 4 2023, 6:49 AM
bartek updated this revision to Diff 31789.Oct 9 2023, 5:34 AM

Rebase

bartek added a child revision: D9413: [blob] Add CLI option to run cleanup task.Oct 9 2023, 5:38 AM
Harbormaster completed remote builds in B23087: Diff 31789.Oct 9 2023, 6:01 AM
michal added a comment.Oct 10 2023, 6:24 AM

If I understand correctly this will try to remove S3 blobs that might not exist (for holders can exist without having a blob uploaded and the logic will run for them too). Is this expected?

services/blob/src/service.rs
304 ↗(On Diff #31789)

orphans could be potentially changed to be a HashMap<String /*blob hash*/, Vec<String /*blob holder*/>>. Or maybe even iterate directly over unchecked_items?

bartek added a comment.Oct 10 2023, 7:50 AM
In D9355#276898, @michal wrote:

If I understand correctly this will try to remove S3 blobs that might not exist (for holders can exist without having a blob uploaded and the logic will run for them too). Is this expected?

This is expected. It causes no harm. Anyway, this is very unlikely scenario, if not impossible, because a DDB row is created after a successful S3 upload.

services/blob/src/service.rs
304 ↗(On Diff #31789)

Iterating directly over unchecked_items makes sense, we could avoid duplicates for free, thanks!

michal accepted this revision.Oct 11 2023, 1:05 AM
This revision is now accepted and ready to land.Oct 11 2023, 1:05 AM
bartek updated this revision to Diff 31918.Oct 11 2023, 2:54 AM

Address feedback: Optimize S3 path iteration

bartek updated this revision to Diff 31921.Oct 11 2023, 2:59 AM

Fix comment

Harbormaster completed remote builds in B23174: Diff 31918.Oct 11 2023, 3:12 AM
Harbormaster completed remote builds in B23177: Diff 31921.Oct 11 2023, 3:15 AM
Closed by commit rCOMM73401a024533: [blob] Delete S3 objects during cleanup (authored by bartek). · Explain WhyOct 16 2023, 1:40 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM73401a024533: [blob] Delete S3 objects during cleanup.

Revision Contents
Changeset List

PathSize
services/
blob/
src/
29 lines
15 lines

Diff 31918

View Options

services/blob/src/s3.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import _maxBy from 'lodash/fp/maxBy.js'; import _maxBy from 'lodash/fp/maxBy.js';
import _orderBy from 'lodash/fp/orderBy.js'; import _orderBy from 'lodash/fp/orderBy.js';
import * as React from 'react'; import * as React from 'react';
import { codeBlockRegex, type ParserRules } from './markdown.js'; import { codeBlockRegex, type ParserRules } from './markdown.js';
import type { CreationSideEffectsFunc } from './messages/message-spec.js'; import type { CreationSideEffectsFunc } from './messages/message-spec.js';
import { messageSpecs } from './messages/message-specs.js'; import { messageSpecs } from './messages/message-specs.js';
import { threadIsGroupChat } from './thread-utils.js'; import { threadIsGroupChat } from './thread-utils.js';
import { useStringForUser } from '../hooks/ens-cache.js'; import { useStringForUser } from '../hooks/ens-cache.js';
import { contentStringForMediaArray } from '../media/media-utils.js';
import { userIDsToRelativeUserInfos } from '../selectors/user-selectors.js'; import { userIDsToRelativeUserInfos } from '../selectors/user-selectors.js';
import { type PlatformDetails, isWebPlatform } from '../types/device-types.js'; import { type PlatformDetails, isWebPlatform } from '../types/device-types.js';
import type { Media } from '../types/media-types.js'; import type { Media } from '../types/media-types.js';
import { import {
type MessageInfo, type MessageInfo,
type RawMessageInfo, type RawMessageInfo,
type RobotextMessageInfo, type RobotextMessageInfo,
type RawMultimediaMessageInfo, type RawMultimediaMessageInfo,
▲ Show 20 Lines • Show All 543 Lines • ▼ Show 20 Lines): CreationSideEffectsFunc<Info> {
invariant(messageSpec, `we're not aware of messageType ${messageType}`); invariant(messageSpec, `we're not aware of messageType ${messageType}`);
invariant( invariant(
messageSpec.useCreationSideEffectsFunc, messageSpec.useCreationSideEffectsFunc,
`no useCreationSideEffectsFunc in message spec for ${messageType}`, `no useCreationSideEffectsFunc in message spec for ${messageType}`,
); );
return messageSpec.useCreationSideEffectsFunc(); return messageSpec.useCreationSideEffectsFunc();
} }
function getPinnedContentFromMessage(targetMessage: RawMessageInfo): string {
let pinnedContent;
if (
targetMessage.type === messageTypes.IMAGES ||
targetMessage.type === messageTypes.MULTIMEDIA
) {
pinnedContent = contentStringForMediaArray(targetMessage.media);
} else {
pinnedContent = 'a message';
}
return pinnedContent;
}
export { export {
localIDPrefix, localIDPrefix,
messageKey, messageKey,
messageID, messageID,
robotextForMessageInfo, robotextForMessageInfo,
createMessageInfo, createMessageInfo,
sortMessageInfoList, sortMessageInfoList,
sortMessageIDs, sortMessageIDs,
rawMessageInfoFromMessageData, rawMessageInfoFromMessageData,
mostRecentMessageTimestamp, mostRecentMessageTimestamp,
usersInMessageInfos, usersInMessageInfos,
combineTruncationStatuses, combineTruncationStatuses,
shimUnsupportedRawMessageInfos, shimUnsupportedRawMessageInfos,
createMediaMessageData, createMediaMessageData,
createMediaMessageInfo, createMediaMessageInfo,
stripLocalIDs, stripLocalIDs,
trimMessage, trimMessage,
createMessageQuote, createMessageQuote,
createMessageReply, createMessageReply,
getMostRecentNonLocalMessageID, getMostRecentNonLocalMessageID,
getMessageTitle, getMessageTitle,
mergeThreadMessageInfos, mergeThreadMessageInfos,
useMessagePreview, useMessagePreview,
useMessageCreationSideEffectsFunc, useMessageCreationSideEffectsFunc,
getPinnedContentFromMessage,
}; };
View Options

services/blob/src/service.rs

// @flow // @flow
import invariant from 'invariant'; import invariant from 'invariant';
import _maxBy from 'lodash/fp/maxBy.js'; import _maxBy from 'lodash/fp/maxBy.js';
import _orderBy from 'lodash/fp/orderBy.js'; import _orderBy from 'lodash/fp/orderBy.js';
import * as React from 'react'; import * as React from 'react';
import { codeBlockRegex, type ParserRules } from './markdown.js'; import { codeBlockRegex, type ParserRules } from './markdown.js';
import type { CreationSideEffectsFunc } from './messages/message-spec.js'; import type { CreationSideEffectsFunc } from './messages/message-spec.js';
import { messageSpecs } from './messages/message-specs.js'; import { messageSpecs } from './messages/message-specs.js';
import { threadIsGroupChat } from './thread-utils.js'; import { threadIsGroupChat } from './thread-utils.js';
import { useStringForUser } from '../hooks/ens-cache.js'; import { useStringForUser } from '../hooks/ens-cache.js';
import { contentStringForMediaArray } from '../media/media-utils.js';
import { userIDsToRelativeUserInfos } from '../selectors/user-selectors.js'; import { userIDsToRelativeUserInfos } from '../selectors/user-selectors.js';
import { type PlatformDetails, isWebPlatform } from '../types/device-types.js'; import { type PlatformDetails, isWebPlatform } from '../types/device-types.js';
import type { Media } from '../types/media-types.js'; import type { Media } from '../types/media-types.js';
import { import {
type MessageInfo, type MessageInfo,
type RawMessageInfo, type RawMessageInfo,
type RobotextMessageInfo, type RobotextMessageInfo,
type RawMultimediaMessageInfo, type RawMultimediaMessageInfo,
▲ Show 20 Lines • Show All 543 Lines • ▼ Show 20 Lines): CreationSideEffectsFunc<Info> {
invariant(messageSpec, `we're not aware of messageType ${messageType}`); invariant(messageSpec, `we're not aware of messageType ${messageType}`);
invariant( invariant(
messageSpec.useCreationSideEffectsFunc, messageSpec.useCreationSideEffectsFunc,
`no useCreationSideEffectsFunc in message spec for ${messageType}`, `no useCreationSideEffectsFunc in message spec for ${messageType}`,
); );
return messageSpec.useCreationSideEffectsFunc(); return messageSpec.useCreationSideEffectsFunc();
} }
function getPinnedContentFromMessage(targetMessage: RawMessageInfo): string {
let pinnedContent;
if (
targetMessage.type === messageTypes.IMAGES ||
targetMessage.type === messageTypes.MULTIMEDIA
) {
pinnedContent = contentStringForMediaArray(targetMessage.media);
} else {
pinnedContent = 'a message';
}
return pinnedContent;
}
export { export {
localIDPrefix, localIDPrefix,
messageKey, messageKey,
messageID, messageID,
robotextForMessageInfo, robotextForMessageInfo,
createMessageInfo, createMessageInfo,
sortMessageInfoList, sortMessageInfoList,
sortMessageIDs, sortMessageIDs,
rawMessageInfoFromMessageData, rawMessageInfoFromMessageData,
mostRecentMessageTimestamp, mostRecentMessageTimestamp,
usersInMessageInfos, usersInMessageInfos,
combineTruncationStatuses, combineTruncationStatuses,
shimUnsupportedRawMessageInfos, shimUnsupportedRawMessageInfos,
createMediaMessageData, createMediaMessageData,
createMediaMessageInfo, createMediaMessageInfo,
stripLocalIDs, stripLocalIDs,
trimMessage, trimMessage,
createMessageQuote, createMessageQuote,
createMessageReply, createMessageReply,
getMostRecentNonLocalMessageID, getMostRecentNonLocalMessageID,
getMessageTitle, getMessageTitle,
mergeThreadMessageInfos, mergeThreadMessageInfos,
useMessagePreview, useMessagePreview,
useMessageCreationSideEffectsFunc, useMessageCreationSideEffectsFunc,
getPinnedContentFromMessage,
}; };