Paths

Table of Contentst

Differential D3612

[services] Backup - Apply reading log chunk in send log reactor
ClosedPublic
Actions

Authored by • karol on Apr 4 2022, 8:02 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Dec 20, 4:04 PM

	Unknown Object (File)
	Fri, Dec 20, 4:04 PM

	Unknown Object (File)
	Fri, Dec 20, 4:04 PM

	Unknown Object (File)
	Fri, Dec 20, 4:04 PM

	Unknown Object (File)
	Fri, Dec 20, 4:00 PM

	Unknown Object (File)
	Thu, Dec 19, 2:33 AM

	Unknown Object (File)
	Wed, Dec 4, 4:59 PM

	Unknown Object (File)
	Wed, Dec 4, 10:56 AM

Subscribers

Details

Reviewers

tomek
• max
varun
• jim

Commits

rCOMM0d5cbefb48f1: [services] Backup - Apply reading log chunk in send log reactor

Summary

Depends on D3531

After reading the user id we want to read log chunks

Test Plan

backup service build

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• karol created this revision.Apr 4 2022, 8:02 AM

Herald added subscribers: • abosh, • benschac, atul and 3 others. · View Herald TranscriptApr 4 2022, 8:02 AM

• karol added a child revision: D3532: [services] Backup - Move Service Blob Client.Apr 4 2022, 8:07 AM

• karol mentioned this in D3531: [services] Backup - Apply reading user id in the reactors.

• karol edited the summary of this revision. (Show Details)

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

• karol added reviewers: tomek, • max, varun, • jim.

• karol added a parent revision: D3531: [services] Backup - Apply reading user id in the reactors.

Services build passed and there are services-only changes here.

Harbormaster completed remote builds in B7840: Diff 11008.Apr 4 2022, 12:31 PM

tomek requested changes to this revision.Apr 5 2022, 9:37 AM

tomek added inline comments.

services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
47–55 ↗	(On Diff #11008)	We probably need a `return` to avoid throwing

This revision now requires changes to proceed.Apr 5 2022, 9:37 AM

• karol requested review of this revision.Apr 6 2022, 12:34 AM

• karol added inline comments.

services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
47–55 ↗	(On Diff #11008)	I don't see what's wrong in throwing here - there should never be a state that's not handled, I think this is a good example of an exceptional/unwanted situation. Side note, effectively there's no difference between returning a grpc error and throwing, both result in terminating the reactor with a proper status (see reactor base class code).

tomek requested changes to this revision.Apr 6 2022, 2:24 AM

tomek added inline comments.

services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
47–53 ↗	(On Diff #11008)
47–55 ↗	(On Diff #11008)	Are you sure we always want to throw when `LOG_CHUNK` state is processed? Because this is what happens now. I think we need a return as the last statement in `case` just like we have it in the other reactor.

This revision now requires changes to proceed.Apr 6 2022, 2:24 AM

add missing return

tomek accepted this revision.Apr 7 2022, 12:20 AM

This revision is now accepted and ready to land.Apr 7 2022, 12:20 AM

Harbormaster completed remote builds in B7945: Diff 11134.Apr 7 2022, 12:26 AM

Closed by commit rCOMM0d5cbefb48f1: [services] Backup - Apply reading log chunk in send log reactor (authored by • karol). · Explain WhyApr 7 2022, 2:42 AM

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM0d5cbefb48f1: [services] Backup - Apply reading log chunk in send log reactor.

Revision Contents
Changeset List

Path

Size

services/

backup/

docker-server/

contents/

server/

src/

Reactors/

server/

SendLogReactor.h

8 lines

Diff 11149

services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h

Loading...