[services][backup] SendLog 3/4 - handle log data
ClosedPublic
Actions

Authored by bartek on Jan 10 2023, 8:40 AM.

Details

Reviewers

varun
tomek
• jon
• max
michal

Commits

rCOMMf250f969aea1: [services][backup] SendLog 3/4 - handle log data

Summary

This diff implements receiving log contents data. By default, log data is going to be directly saved in database.
Each received data chunk is appended to the buffer. If the log item becomes too large to be stored in DynamoDB,
then log contents is moved to Blob service storage and further chunks are uploaded there directly.

Depends on D6212

Test Plan

The service builds and starts. Logic of this diff can be tested altogether with the child diff - see the description there.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Jan 10 2023, 8:40 AM

bartek held this revision as a draft.

Herald added subscribers: atul, ashoat. · View Herald TranscriptJan 10 2023, 8:40 AM

bartek added a child revision: D6214: [services][backup] SendLog 4/4 - save and finish.Jan 10 2023, 8:48 AM

Harbormaster completed remote builds in B15224: Diff 20754.Jan 10 2023, 8:55 AM

bartek published this revision for review.Jan 11 2023, 1:31 PM

bartek added a reviewer: michal.

tomek accepted this revision.Jan 12 2023, 7:29 AM

tomek added inline comments.

services/backup/src/service/handlers/send_log.rs
127–133 ↗	(On Diff #20754)	This doesn't seem to be properly formatted (the curly bracket)
134–158 ↗	(On Diff #20754)	Can we make this symmetric by introducing `move_to_blob` function?
141 ↗	(On Diff #20754)	Why do we have to provide `holder` twice?

bartek planned changes to this revision.Jan 12 2023, 10:02 AM

bartek added inline comments.

services/backup/src/service/handlers/send_log.rs
127–133 ↗	(On Diff #20754)	`cargo fmt` must be wrong ;) It forces the `{` to be in new line
134–158 ↗	(On Diff #20754)	Sure, will do
141 ↗	(On Diff #20754)	my bad - I misread the ugly C++ code, it should be `log_hash`

Worked around curly bracket formatting ;)
Fixed incorrect hash provided to put client
Extracted the ensure_size_constraints() function which both checks logs size and moves to blob if it's too big. At first I tried extracting only move_to_blob() but this required either code duplication (params unpacking) or violated borrow checker rules, eventually nasty cloning.

Harbormaster completed remote builds in B15318: Diff 20893.Jan 12 2023, 11:39 AM

tomek accepted this revision.Jan 16 2023, 6:33 AM

• jon accepted this revision.Jan 17 2023, 11:24 AM

varun accepted this revision.Jan 19 2023, 8:25 AM

This revision is now accepted and ready to land.Jan 19 2023, 8:25 AM

Rebase
Set persistence to blob regardless of blob client being created. Otherwise, when hash already exists, persistence could be switched back to DB and cause trouble because it's already too large.

Harbormaster completed remote builds in B15470: Diff 21092.Jan 19 2023, 9:57 AM

Closed by commit rCOMMf250f969aea1: [services][backup] SendLog 3/4 - handle log data (authored by bartek). · Explain WhyJan 19 2023, 10:08 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMMf250f969aea1: [services][backup] SendLog 3/4 - handle log data.