Repository navigation
S3C-11599 crrExistingObjects: write the multi-destination format for every bucket rule format - #418
nicolas2bert wants to merge 1 commit into
Conversation
Hello nicolas2bert,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect Jira projectThe Jira issue S3C-11599 specified in the source |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| enabled: r.Status === 'Enabled', | ||
| prefix: r.Filter?.Prefix ?? r.Prefix ?? '', | ||
| storageClass: r.Destination.StorageClass, | ||
| storageClass: r.Destination.StorageClass || this.siteName, |
There was a problem hiding this comment.
With mixed rules, this sends objects to the wrong bucket. Example: r1 has no StorageClass (default site dr, bucket dr-bucket), r2 has StorageClass sf (bucket sf-bucket), and SITE_NAME=sf is set to requeue sf. r1 is then mapped to site sf, so resolveBackends returns a candidate {site: sf, destination: dr-bucket}. No existing backend matches it, so NEW picks it and backbeat copies the object to dr-bucket through site sf. Before this PR, r1 had no site and resolveBackends ignored it. Only use SITE_NAME for rules without StorageClass when no other rule names SITE_NAME, e.g. !this._getConfiguredSites(Rules).includes(this.siteName). Please add a unit test for this case.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/1.19 #418 +/- ##
====================================================
+ Coverage 47.14% 47.23% +0.09%
====================================================
Files 90 90
Lines 6809 6798 -11
Branches 1460 1460
====================================================
+ Hits 3210 3211 +1
+ Misses 3543 3532 -11
+ Partials 56 55 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ef8c01a to
5890841
Compare
| return !replicationInfo?.destination | ||
| || candidate.destination === replicationInfo.destination; | ||
| } | ||
| return existing.destination === candidate.destination && existing.role === candidate.role; |
There was a problem hiding this comment.
Matching on role breaks requeues after the bucket Role changes (rotated role, or a new Destination.Account). The existing backend keeps the old role, so it no longer matches the candidate. Result: TARGET_REPLICATION_STATUS=FAILED/COMPLETED skip the object silently, and NEW adds a second backend for the same site and bucket. That goes against step 2 ("requeue with the current bucket and role", S3UTILS-208). Match on site and destination only. The picked backend gets the candidate's role anyway.
| return existing.destination === candidate.destination && existing.role === candidate.role; | |
| return existing.destination === candidate.destination; |
5890841 to
b7b5bd5
Compare
| // Example: NEW picks dr2. COMPLETED would pick sf. | ||
| const pairs = candidateBackends.map(candidate => { | ||
| const existing = existingBackends.find(e => this._isSameBackend(e, candidate, prev)); | ||
| const selected = this._statusShouldBeProcessed(existing?.status); |
There was a problem hiding this comment.
TARGET_REPLICATION_STATUS=REPLICA now checks the status of the matched backend, not the object status. In the documented DR failback (README "Re-sync a DR site back to the primary site"), the candidate is the primary site, and the replica has no backend for it (only dr=REPLICA). So existing is undefined, REPLICA never matches, and every replica is skipped without a word. Match REPLICA on the object instead, e.g. filter === 'REPLICA' ? prev?.status === 'REPLICA' && !existing : ..., and add a unit test for the failback case.
b7b5bd5 to
75908d5
Compare
| // Example: NEW picks dr2. COMPLETED would pick sf. | ||
| const pairs = candidateBackends.map(candidate => { | ||
| const existing = existingBackends.find(e => this._isSameBackend(e, candidate, prev, true)) | ||
| || existingBackends.find(e => this._isSameBackend(e, candidate, prev)); |
There was a problem hiding this comment.
The role-free fallback can take a backend that another candidate already matched by role. Example: rule A (account 111) replicated the object to site sf, bucket d1. Then rule B (account 222, same site and bucket) is added. With NEW, candidate B falls back to A's COMPLETED backend, so it isn't picked and B never gets a backend. In the fallback, skip existing backends whose role matches a candidate's role (or that another pair already claimed), and add a unit test for this case.
75908d5 to
cd61e91
Compare
…rmat crrExistingObjects picked its write path from the bucket rule format. Rules without Filter (V1) got the old single-destination format: a new backend had no bucket and no role. Since S3C 10.1, cloudserver writes the multi-destination format for every bucket, whatever the rule format, so backbeat failed with BadRole and the object stayed PENDING (S3C-11599). The rule format only changes where the prefix is. s3utils 1.19 runs on S3C 10.1 and later only. - one write path for every bucket: bucket and role per backend, from the bucket rules (arsenal resolveBackends, like cloudserver on a PUT) - top-level status PENDING: the backbeat queue populator only queues PENDING objects. PROCESSING left objects of V2 buckets never replicated (S3C-11596) - compute content when it is empty (object put before the rule) - rule without StorageClass: SITE_NAME is the S3C default site. Fails with "missing SITE_NAME" when it's not set - SITE_NAME unset: every site of the rules, instead of the site of the first rule - keys outside every rule prefix are skipped: backbeat refuses them anyway - match object backends on site, bucket and role: one site can have several buckets since S3C 10.1. Backends written by S3C 9.5 match on the site - don't add a rule site that TARGET_REPLICATION_STATUS doesn't pick, keep the backends this run doesn't pick as they are - top-level role: source role from the bucket config. Keep the object role and destination while a backend written by S3C 9.5 is kept: backbeat needs them for it - FORCE_USING_CONFIGURATION is ignored: the bucket config is always used (S3UTILS-208) - warn when SITE_NAME is not a site of the bucket rules - keep the unknown-site check (S3UTILS-250), the SITE_NAME requeue of a site the rules don't name anymore, and isReplica - STORAGE_TYPE is ignored
cd61e91 to
5ef47db
Compare
problem
On S3C 10.1, crrExistingObjects marks objects for replication, but backbeat never replicates them. The objects stay PENDING.
The script chose how to write the object metadata from the format of the bucket replication rules. Both ways were wrong on S3C 10.1:
Since S3C 10.1, cloudserver writes the same format for every bucket, whatever the rule format: one backend per destination, each with its own bucket and role. The rule format only changes where the prefix is (Prefix or Filter.Prefix).
Example used below:
fix
We now write the object metadata like cloudserver does on a PUT (arsenal resolveBackends), whatever the rule format:
Example: 'docs/a.pdf' matches r1 and r2, so we get site sf, bucket backup-sf, and site dr2, bucket backup-dr2.
Example: site sf, bucket backup-sf is COMPLETED. Site dr2, bucket backup-dr2 has no backend, so NEW picks it.
Example: we add site dr2, bucket backup-dr2, role crr-dst, PENDING. The site sf backend stays COMPLETED.
We also fix two cases:
Example: say r2 is a V2 bucket replication rule with no StorageClass, and we run with SITE_NAME=dr2 and NEW. 'docs/b.pdf' now gets site dr2, bucket backup-dr2. Before, we skipped it.
Example: say r2 sends to site sf, bucket backup-sf-2. 'docs/a.pdf' has site sf, bucket backup-sf, COMPLETED, and site sf, bucket backup-sf-2, FAILED. A FAILED run now retries site sf, bucket backup-sf-2 only. Before, we matched on the site only, so we saw one status for site sf, and could retry the wrong bucket, or none.
behavior changes
They are all in the README, with examples.
Before: the format we wrote depended on the rule format. With V1 bucket replication rules, we wrote the S3C 9.5 format.
Why: since S3C 10.1, backbeat expects each backend to carry its own bucket and role, and S3C 9.5 doesn't understand that. Keeping both formats means keeping the write path that broke (S3C-11599).
Before: with V1 bucket replication rules, we only requeued the site of the first rule, with a warning. With V2 bucket replication rules, nothing changes: we already requeued every site.
Example: we run with NEW. 'docs/b.pdf' matches r1 and r2, so we now add site sf, bucket backup-sf, and site dr2, bucket backup-dr2. Before, with V1 bucket replication rules, we only added site sf.
Why: the rule format only changes where the prefix is, so it must not change which sites we requeue. The old V1 code took one site per bucket as input, so without SITE_NAME it used the site of the first rule. Now one function handles both formats with resolveBackends, like cloudserver on a PUT.
Before: with V1 bucket replication rules, we fell back to the site of the first rule, and only failed when the first rule had no StorageClass. With V2 bucket replication rules, we didn't fail: we ignored the rule.
Example: say r2 has no StorageClass, and SITE_NAME is not set. The run now fails. Before, with V1 bucket replication rules, we used site sf from r1 for every key, so 'docs/b.pdf' never went to the S3C default site.
Why: a rule without StorageClass goes to the S3C default site. That site is in the federation config, which the script can't read, so only SITE_NAME gives it. Falling back to the site of the first rule sent objects to the wrong site: we fail instead of guessing.
Before: with V1 bucket replication rules, the run stopped at the first object with no backend for SITE_NAME. With V2 bucket replication rules, we skipped the objects without a warning.
Example: we run with SITE_NAME=dr3 and NEW. We skip 'docs/a.pdf' and 'docs/b.pdf', and log "SITE_NAME is not a site of the bucket rules". Before, with V1 bucket replication rules, the run stopped at 'docs/a.pdf'.
Why: no rule gives a backend for SITE_NAME, so we have nothing to write. The stop protects objects we are about to write a wrong site to (S3UTILS-250): here there are none. It also stopped runs for nothing: a FAILED run stopped on objects with nothing to retry. The warning says why few or no objects are updated.
Before: with V1 bucket replication rules, we marked them PENDING, and backbeat refused them ("replication disabled for object"). With V2 bucket replication rules, we already skipped them.
Example: say r1 has Prefix 'photos/' instead of ''. 'notes.txt' matches no rule, so we skip it. Before, with V1 bucket replication rules, we marked it PENDING on site sf, and it stayed PENDING.
Why: bug. Backbeat refuses these objects, so they stayed PENDING forever. Cloudserver and the old V2 code already skipped them.
Before: we kept the role the object was sent with, unless FORCE_USING_CONFIGURATION was set.
Example: say the bucket Role is now '...:role/crr-src,...:role/crr-dst-new'. We run with COMPLETED. 'docs/a.pdf' gets site sf, bucket backup-sf, role crr-dst-new, PENDING. Before, with V1 bucket replication rules and without FORCE_USING_CONFIGURATION, backbeat sent it again with the old role crr-dst.
Why: keeping the old bucket and role made backbeat fail with BadRole after a config change (S3UTILS-208). Each backend now gets its bucket and role from the current rule, so the flag has nothing left to do.
Before: we matched backends on the site only. NEW skipped the object, because the site already had a backend. COMPLETED with FORCE_USING_CONFIGURATION=true sent it to the new bucket, but replaced the backend for the old bucket.
Example: say r1 now uses site sf, bucket backup-sf-new. We run with NEW. 'docs/a.pdf' gets site sf, bucket backup-sf-new, PENDING, and keeps site sf, bucket backup-sf, COMPLETED. Before, NEW skipped it.
Why: since S3C 10.1, one site can have several buckets, so we match backends on site, bucket and role, not on the site only. A new bucket is a new destination, so NEW picks it. Before, overwriting the old backend lost the record of the copy in the old bucket.
Before: with V1 bucket replication rules, we wrote it to the top-level storageType of the object.
Why: the S3C 10.1 format has no top-level storageType: there is nowhere to write it.
Issue: S3C-11599, S3C-11596