Page MenuHomePhabricator
Differential D9350

[blob] Add struct for representing unchecked blobs
ClosedPublic
Actions

Authored by bartek on Oct 4 2023, 3:02 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Oct 28, 12:20 PM
Unknown Object (File)
Sun, Oct 27, 4:25 PM
Unknown Object (File)
Fri, Oct 18, 4:00 AM
Unknown Object (File)
Thu, Oct 17, 10:13 PM
Unknown Object (File)
Sat, Oct 12, 9:26 PM
Unknown Object (File)
Sat, Oct 12, 2:34 PM
Unknown Object (File)
Sep 28 2024, 12:24 AM
Unknown Object (File)
Sep 28 2024, 12:24 AM
View All Files
Subscribers
ashoat
will

Details

Reviewers
tomek
michal
varun
• jon
Commits
rCOMMbec5074be99e: [blob] Add struct for representing unchecked blobs
Summary

Added a struct for representing unchecked items in database. It greatly simplifies the cleanup job.
It is used in the next two diffs

Depends on D9349

Test Plan

Code compiles, implementation is tested in the next two diffs.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Oct 4 2023, 3:02 AM
bartek held this revision as a draft.
Herald added a reviewer: • jon. · View Herald TranscriptOct 4 2023, 3:02 AM
Herald added subscribers: will, ashoat. · View Herald Transcript
bartek added a child revision: D9351: [blob] Implement cleanup helper functions.Oct 4 2023, 3:04 AM
Harbormaster completed remote builds in B22958: Diff 31622.Oct 4 2023, 3:19 AM
bartek published this revision for review.Oct 4 2023, 3:20 AM
bartek updated this revision to Diff 31786.Oct 9 2023, 5:33 AM

Rebase. Add debug derive

Harbormaster completed remote builds in B23084: Diff 31786.Oct 9 2023, 5:52 AM
tomek accepted this revision.Oct 10 2023, 4:46 AM
tomek added 1 blocking reviewer(s): michal.
michal accepted this revision.Oct 10 2023, 6:09 AM
michal added inline comments.
services/blob/src/service.rs
232–235 ↗(On Diff #31786)

From looking at the later code it seems like we could just have:

struct UncheckedItem {
  has_hash: bool,
  holders: Vec<String>,
};

but it's fine the way it is now.

If dynamodb allowed deletion with just partition key we could even just have has_holders: bool...

277 ↗(On Diff #31786)

Can you add a comment (or a type alias) that explains what a String is in this case?

This revision is now accepted and ready to land.Oct 10 2023, 6:09 AM
bartek added inline comments.Oct 10 2023, 7:33 AM
services/blob/src/service.rs
232–235 ↗(On Diff #31786)

Theoretically yes, at some point I even considered that. However, I wanted each UncheckedItem to be independent: to contain full information about blob, regardless if it's inside or outside a map-like collection.

bartek updated this revision to Diff 31915.Oct 11 2023, 2:47 AM

Address feedback: Add type alias for blob hash, use ref instead of cloning.

Harbormaster completed remote builds in B23171: Diff 31915.Oct 11 2023, 3:05 AM
Closed by commit rCOMMbec5074be99e: [blob] Add struct for representing unchecked blobs (authored by bartek). · Explain WhyOct 16 2023, 1:40 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMMbec5074be99e: [blob] Add struct for representing unchecked blobs.

Revision Contents
Changeset List

PathSize
services/
blob/
src/
69 lines

Diff 31622

View Options

services/blob/src/service.rs

Loading...