Page MenuHomePhabricator

[services] Backup - Add server reactor base classes - write reactor
ClosedPublic

Authored by karol on Mar 24 2022, 1:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:04 PM
Unknown Object (File)
Fri, Dec 20, 8:01 PM

Details

Summary

Depends on D3512

This class is moved and renamed for a better folder structure.
As we need to add client reactors in order to be able to connect to the blob service, we need to distinguish server reactors from client reactors.

Test Plan
cd services
yarn run-backup-service

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

karol edited the summary of this revision. (Show Details)
karol edited the test plan for this revision. (Show Details)
karol added reviewers: max, jim, varun, tomek.
karol edited the summary of this revision. (Show Details)

update

karol retitled this revision from [services] Backup - Add server reactor base classes - write reactor to [draft] [services] Backup - Add server reactor base classes - write reactor.Mar 24 2022, 2:47 AM
tomek added inline comments.
services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerWriteReactorBase.h
61

Why do we call directly Finish here while in Read reactor we have terminate method that sets the status. Why status is not needed in this reactor, or why it is needed in that reactor?

This revision is now accepted and ready to land.Mar 24 2022, 5:32 AM
services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerWriteReactorBase.h
61

you're right, I'm going to add terminate here for the consistency

It is accepted as a draft so I removed the draft label and I'm requesting changes once again to keep the appropriate order of events.

karol retitled this revision from [draft] [services] Backup - Add server reactor base classes - write reactor to [services] Backup - Add server reactor base classes - write reactor.Mar 25 2022, 6:59 AM
tomek added inline comments.
services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerWriteReactorBase.h
61–66 ↗(On Diff #10677)

Could you explain what is the difference and purpose of writeResponse and StartWrite?

85–89 ↗(On Diff #10677)

Do we need to wrap it with try-catch when NextWrite also uses try-catch internally?

This revision is now accepted and ready to land.Mar 28 2022, 9:39 AM
services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerWriteReactorBase.h
61–66 ↗(On Diff #10677)

StartWrite is a method provided by the grpc in the grpc::ServerWriteReactor class. It tells the grpc to start writing a response to the client.

writeResponse is a method introduced by me. Its purpose is to be implemented in derived classes so we can generate sequential responses that will be sent to the client in derived classes.

85–89 ↗(On Diff #10677)

You're right, this is redundant