dr clean view : do not set non localized versions as master versions - #2708
SylvainSenechal wants to merge 3 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.6 #2708 +/- ##
===================================================
+ Coverage 74.76% 74.80% +0.04%
===================================================
Files 227 227
Lines 18650 18670 +20
Branches 3864 3900 +36
===================================================
+ Hits 13943 13967 +24
+ Misses 4702 4698 -4
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
38112f5 to
6d3f6e8
Compare
| * @param {Function} cb callback | ||
| * @return {undefined} | ||
| */ | ||
| putObjectVerCase4( |
There was a problem hiding this comment.
Mongo processor will only use VerCase4 to put non localized objects :
https://github.com/scality/backbeat/blob/57765e8f67e61fd7fb2ba36facbda6cc70054a31/extensions/mongoProcessor/MongoQueueProcessor.js#L562
And bootstrap writes directly in mongo without using the Arsenal functions.
So this PR ends up only change put ver case 4.
I could add something like this in the other put functions :
if (this.nonLocalizedLocations.has(objVal.dataStoreName)) {
return err / log err / log warn
}
because in theory, these put function could later be used to put non localized objects (like if we were to start supporting non versioning objects, we would not use ver case 4 but another), but I don't think we wanna do that
There was a problem hiding this comment.
Update, I did change the other functions, but for some of them the change can be discussed
There was a problem hiding this comment.
And second update, technically, the mongo processor can also call putObjectNoVer, but only for null version, but it could happen in theory : User creates a non versioned bucket and writes objects, then set bucket versioning, then setup disaster recovery on the bucket.
d2bfe88 to
ba915f1
Compare
Hello sylvainsenechal,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
6d3f6e8 to
8739266
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
8739266 to
b34f39f
Compare
| * @param {Object} objVal object metadata | ||
| * @return {Boolean} true if the version is non-localized | ||
| */ | ||
| isNonLocalized(objVal: ObjectMDData): boolean { |
There was a problem hiding this comment.
Nit: We can discuss the naming of this, because in the functions later we end up with stuff like
if not not
if (!this.isNonLocalized(objVal)) {
| const versionId = generateVersionId(this.instanceId, this.replicationGroupId); | ||
| objVal.versionId = versionId; | ||
| const masterKey = formatMasterKey(objName, params.vFormat); | ||
| // the master is the only document this path writes : a non-localized version |
There was a problem hiding this comment.
I did this for putObjectNoVer, putObjectWithCond and putObjectVerCase2, but it's really debatable
Logging an error but continuing...
These 3 functions only write masters. But also from what I found, they won't be called by the mongo processor, so it's more of a security, but as it is it's just not really useful.
Could return an error instead, but currently these errors won't be handled either, or just not do anything and consider that these functions will never be called with a non localized object
| * @returns {Promise} A promise that resolves when the operation is complete. The promise is rejected with an error | ||
| * if there is an issue with the operation. | ||
| */ | ||
| putObjectNoVer( |
There was a problem hiding this comment.
As I wrote in another comment, this function may be a bit more tricky :
- it can be called by mongo processor for null versions (the scenario would be : User creates non versioned bucket and writes objects, then turn versioning bucket on, then setup disaster recovery)
- In that scenario, we need to write the master version even if its non localized otherwise we just dont store anything for the object..
- It's probaly still fine because regardless of what we do, non localized entries will still be hidding by clean read
b34f39f to
c6745fd
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // the master is the only document this path writes : a non-localized version | ||
| // reaching this function is unexpected | ||
| if (this.isNonLocalized(value)) { | ||
| log.error('putObjectNoVer: writing a non-localized version as master', { |
There was a problem hiding this comment.
log.error is too severe here. As you noted in the PR comments, the mongo processor can reach putObjectNoVer for null versions (non-versioned bucket → enable versioning → setup DR). That's a legitimate production path, not an unexpected error. Using error level would trigger alerts and noise for normal operation.
Consider log.warn (or log.info) since the write is intentional and the clean-read filter still hides the entry.
| log.error('putObjectNoVer: writing a non-localized version as master', { | |
| log.warn('putObjectNoVer: writing a non-localized version as master', { |
Isssue: ARSN-618
|
I'll lint later |
2 participants
ISSUE: ARSN-618
Implementing Clean View for object whose metadata are still sitting in the source location.
The previous PR ARSN-617 dealt with Clean View : Reads
This one is for Clean View : Writes
When mongo processor sends a put request, the mongo interface will now check if the object's location is localized or not. Depending on that it will (or not) update the master : A non localized object version data cannot be promotoed to be a master (edit: except in one particular case highlighted in the review comment 🧐 )