Details

services/backup/docker-server/contents/server/src/Constants.h
33–34	This comment should explain why smaller size allows us to do so. Or it could explain why it is not possible when the size is greater.
services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
77–82	Have you tested this code? We always throw here: if persistenceMethod = PersistenceMethod::UNKNOWN we throw because persistenceMethod != PersistenceMethod::BLOB if persistenceMethod = PersistenceMethod::BLOB we throw because persistenceMethod != PersistenceMethod::UNKNOWN in all the other cases we throw because persistenceMethod != PersistenceMethod::UNKNOWN

This revision now requires changes to proceed.Apr 1 2022, 4:38 AM

services/backup/docker-server/contents/server/src/Constants.h
33–34	ok
services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
77–82	I saw this issue before, there should be `&&`, not `\|\|`. I thought I updated this but apparently, I did not, thanks for spotting this!

update comment, fix condition

tomek requested changes to this revision.Apr 4 2022, 4:21 AM

tomek added inline comments.

services/backup/docker-server/contents/server/src/Constants.h
33–37 ↗	(On Diff #10986)	Please use sentences (capital letters and dots).
services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
68–88 ↗	(On Diff #10986)	I'm not sure if I understand the idea behind this, but this doesn't seem to be correct. Let's suppose we want to persist two chunks, one bigger than `LOG_DATA_SIZE_DATABASE_LIMIT` and one smaller. After the first chunk, we set `this->persistenceMethod = PersistenceMethod::BLOB`. When processing the second, the first `if` condition is true, and we throw because `this->persistenceMethod != PersistenceMethod::UNKNOWN`. Am I missing something? As a side note, it's better to avoid throwing when we can recover from invalid state. In this case, if we started writing to the db and then the size limit is exceeded, we can move the data to blob and write the rest there.

This revision now requires changes to proceed.Apr 4 2022, 4:21 AM

update logic

services/backup/docker-server/contents/server/src/Constants.h
33–37 ↗	(On Diff #10986)	ok
services/backup/docker-server/contents/server/src/Reactors/server/SendLogReactor.h
68–88 ↗	(On Diff #10986)	You are right, there's a logic error here, I'm going to fix this. As a side note, it's better to avoid throwing when we can recover from invalid state. In this case, if we started writing to the db and then the size limit is exceeded, we can move the data to blob and write the rest there. This would make sense but I think not in this particular case. The idea here is that storing in DB is for one-chunk operations only. If we ever receive more than one chunk, we should never store the bytes in the DB.