[services] Backup - return log id of the newly created log to the client
ClosedPublic
Actions

Authored by • karol on May 19 2022, 10:00 AM.

Details

Reviewers

tomek
ashoat

Commits

Summary

Depends on D4033

In order to Be able to send attachments for certain logs, we should first get new logs ids from the backup service.

Test Plan

cd services
yarn run-blob-service-dev-mode

cd services
yarn run-backup-service-dev-mode

git clone https://github.com/karol-bisztyga/grpc-playground.git
cd grpc-playground
git checkout backup-async
./build.sh
./cmake/build/bin/client

Then in terminal 3 create a new backup

Then in terminal 3 create a new log

And see that you obtained a new log id.

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

• karol requested review of this revision.May 19 2022, 10:05 AM

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

• karol added a reviewer: tomek.

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

Adding @ashoat as the proto is changed

services/backup/src/Reactors/server/CreateNewBackupReactor.cpp
94–95 ↗	(On Diff #12918)	Shouldn't we update these in a diff that moved the tools?

ashoat added inline comments.

native/cpp/CommonCpp/grpc/protos/backup.proto
55 ↗	(On Diff #12918)	Is the `logID` a timestamp? I wonder if it's better to be more specific here and call this `logCheckpoint` or `logTimestamp` or something
services/backup/src/Reactors/server/CreateNewBackupReactor.cpp
94–95 ↗	(On Diff #12918)	Agree, looks like something may have gone wrong with Git here?

This revision is now accepted and ready to land.May 23 2022, 5:50 PM

native/cpp/CommonCpp/grpc/protos/backup.proto
55 ↗	(On Diff #12918)	Log id != log timestamp. Log ID generating function: std::string SendLogReactor::generateLogID(const std::string &backupID) { return backupID + tools::ID_SEPARATOR + std::to_string(tools::getCurrentTimestamp()); } I'm not going to do any updates here as you were wrong with your assumption. If you still think the naming can be better here, let me know. Nevertheless, I think the naming is at its best here. We have a function `generateLogID` and then we want to return it to the client. It is stored in the DB as the `logID`. Why adding such a confusion here?

update

ashoat added inline comments.

native/cpp/CommonCpp/grpc/protos/backup.proto
56 ↗	(On Diff #13045)	Thanks for clarifying! I can see it's not a timestamp, but it is monotonically increasing, so it might count as a "checkpoint". I guess in this case we can treat the `SendLog` as an independent operation, but if/when we do have multiple logs queued up, it might make sense to checkpoint their upload, just like we checkpoint their download.