Paths

Table of Contentst

Differential D4246

[services] Backup - Send attachments from pull backup reactor
ClosedPublic
Actions

Authored by • karol on Jun 13 2022, 6:50 AM.

Tags

None

Referenced Files

	F3299472: D4246.id13882.diff
	Sun, Nov 17, 12:29 PM

	Unknown Object (File)
	Sat, Nov 2, 6:04 PM

	Unknown Object (File)
	Sat, Oct 26, 5:05 AM

	Unknown Object (File)
	Sat, Oct 26, 5:05 AM

	Unknown Object (File)
	Sat, Oct 26, 5:05 AM

	Unknown Object (File)
	Sat, Oct 26, 5:05 AM

	Unknown Object (File)
	Sat, Oct 26, 5:05 AM

	Unknown Object (File)
	Sat, Oct 26, 4:56 AM

Subscribers

Details

Reviewers

Commits

rCOMMb1802f5c4254: [services] Backup - Send attachments from pull backup reactor

Summary

Depends on D4245

We want to send attachment holders for both logs and compactions.

The data is sent in the following order:

compaction data
compaction holders
for log in logs
- log data
- log chunk

Test Plan

Services will still build properly. This change is eventually tested on the end of this stack when the logic for the backup test is added

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• karol created this revision.Jun 13 2022, 6:50 AM

Herald added subscribers: • abosh, atul, • adrian and 2 others. · View Herald TranscriptJun 13 2022, 6:50 AM

• karol edited the summary of this revision. (Show Details)Jun 13 2022, 7:09 AM

• karol edited the test plan for this revision. (Show Details)

• karol added reviewers: tomek, varun.

• karol added a parent revision: D4245: [services] Backup - Add current id and attachment holders.

• karol removed a reviewer: varun.

• karol added a child revision: D4247: [services] Tests - Add add_attachments.Jun 13 2022, 7:13 AM

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 13 2022, 10:01 AM

Harbormaster failed remote builds in B9722: Diff 13467!

CI failed on an unrelated stage - key server
CI services build - no space left on device

Harbormaster completed remote builds in B9722: Diff 13467.Jun 14 2022, 3:51 AM

Overall, this code is becoming really hard to read due to excessive conditional logic. We should consider choosing a different abstraction, e.g. a state machine - by doing this we would have clearly defined logic and state transitions.

This revision is now accepted and ready to land.Jun 20 2022, 4:08 AM

In D4246#121225, @palys-swm wrote:

Overall, this code is becoming really hard to read due to excessive conditional logic. We should consider choosing a different abstraction, e.g. a state machine - by doing this we would have clearly defined logic and state transitions.

Just commenting to say that I agree

Ok, I created a task for this https://linear.app/comm/issue/ENG-1291/make-reactors-code-easier-to-read

separate log id from backup id

Harbormaster failed remote builds in B10042: Diff 13882!Jun 28 2022, 4:56 AM

• karol added a child revision: D4376: [services] Tests - Separate backup id from log id for pull backup.Jun 28 2022, 5:01 AM

• karol removed a child revision: D4247: [services] Tests - Add add_attachments.

retrigger CI

Harbormaster completed remote builds in B10142: Diff 14012.Jun 30 2022, 7:24 AM

Closed by commit rCOMMb1802f5c4254: [services] Backup - Send attachments from pull backup reactor (authored by • karol). · Explain WhyJul 8 2022, 2:30 AM

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMMb1802f5c4254: [services] Backup - Send attachments from pull backup reactor.

Revision Contents
Changeset List

Path

Size

services/

backup/

src/

Reactors/

server/

PullBackupReactor.h

5 lines

PullBackupReactor.cpp

71 lines

Diff 14324

services/backup/src/Reactors/server/PullBackupReactor.h

Loading...

services/backup/src/Reactors/server/PullBackupReactor.cpp

Loading...