[services][backup] Implement blob Get client
ClosedPublic
Actions

Authored by bartek on Jan 4 2023, 9:09 AM.

Details

Reviewers

varun
tomek
• jon
• max

Commits

rCOMMbbb809c9b2fa: [services][backup] Implement blob Get client

Summary

This diff implements the blob Get client logic. Added extensive in-code comments describing its working principle.

This code is an improved and simplified version of old blob_client: (link) - I got rid of custom Tokio runtimes and mutexes, which caused problems in the previous implementation.

Depends on D6165

Test Plan

This code is tested in subsequent diffs where it is used in backup service ednpoints.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Jan 4 2023, 9:09 AM

bartek held this revision as a draft.

Herald added subscribers: atul, ashoat. · View Herald TranscriptJan 4 2023, 9:09 AM

bartek added a child revision: D6167: [services][backup] Implement blob Put client.Jan 4 2023, 9:11 AM

bartek published this revision for review.Jan 4 2023, 9:23 AM

Harbormaster completed remote builds in B15087: Diff 20578.Jan 4 2023, 9:24 AM

Rebase

Harbormaster completed remote builds in B15111: Diff 20606.Jan 5 2023, 3:55 AM

Looks a lot better than the previous implementation!

services/backup/src/blob/get_client.rs
57 ↗	(On Diff #20606)	Could you explain why do we call `in_current_span`?

bartek added inline comments.Jan 9 2023, 5:19 AM

services/backup/src/blob/get_client.rs
57 ↗	(On Diff #20606)	This is related to `tracing` concept of Span. When we spawn a Tokio task inside a span (in this case `"get_client"`), by default the span will not include the code inside that task. Calling `in_current_span` makes any log produced inside the task to be marked as `get_client` instead of a new, empty span. The docs example should better explain what I mean.