Never garbage-collect data on an isCRR location - #2828
Merged
Conversation
Contributor
Hello francoisferrand,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 5 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.6 #2828 +/- ##
===================================================
+ Coverage 76.37% 76.70% +0.32%
===================================================
Files 205 205
Lines 14215 14225 +10
===================================================
+ Hits 10857 10911 +54
+ Misses 3348 3304 -44
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
francoisferrand
marked this pull request as ready for review
August 24, 2026 08:31
francoisferrand
force-pushed
the
improvement/BB-813
branch
from
August 24, 2026 08:48
750b874 to
c5a5e88
Compare
francoisferrand
marked this pull request as draft
August 24, 2026 12:39
francoisferrand
marked this pull request as ready for review
August 24, 2026 12:40
francoisferrand
marked this pull request as draft
August 26, 2026 07:52
Contributor
Now fixed, rebase and enjoy :) |
delthas
added a commit
that referenced
this pull request
Aug 27, 2026
Data on an `isCRR` location belongs to a remote site: it may be read, but never deleted, and a version whose data still lives there has not been localized yet. Several places need to ask that question. Lifted verbatim from BB-813 (#2828), which introduces the same helper and carries its unit test and the location fixture. Kept byte-identical so that whichever branch lands second has this commit dropped as already applied, rather than conflicting. Issue: BB-811
delthas
added a commit
that referenced
this pull request
Aug 31, 2026
Data on an `isCRR` location belongs to a remote site: it may be read, but never deleted, and a version whose data still lives there has not been localized yet. Several places need to ask that question. Lifted verbatim from BB-813 (#2828), which introduces the same helper and carries its unit test and the location fixture. Kept byte-identical so that whichever branch lands second has this commit dropped as already applied, rather than conflicting. Issue: BB-811
francoisferrand
force-pushed
the
improvement/BB-813
branch
from
September 4, 2026 22:58
fc65001 to
3fea783
Compare
francoisferrand
changed the base branch from
development/9.6
to
improvement/BB-816-skip-non-localized-en
September 4, 2026 23:00
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
francoisferrand
force-pushed
the
improvement/BB-813
branch
3 times, most recently
from
September 4, 2026 23:46
7e46ca1 to
226a14c
Compare
francoisferrand
marked this pull request as ready for review
September 4, 2026 23:57
DarkIsDude
approved these changes
Sep 7, 2026
francoisferrand
force-pushed
the
improvement/BB-813
branch
from
September 8, 2026 07:50
226a14c to
777d6f9
Compare
maeldonn
approved these changes
Sep 8, 2026
delthas
added a commit
that referenced
this pull request
Sep 10, 2026
Data on an `isCRR` location belongs to a remote site: it may be read, but never deleted, and a version whose data still lives there has not been localized yet. Several places need to ask that question. Lifted verbatim from BB-813 (#2828), which introduces the same helper and carries its unit test and the location fixture. Kept byte-identical so that whichever branch lands second has this commit dropped as already applied, rather than conflicting. Issue: BB-811
In a clean room, objects exist locally in metadata but their data still lives on the production (isCRR) location. Localizing such an object reuses the lifecycle transition pipeline, which normally garbage-collects the from-location once the copy is merged into metadata: against an isCRR source that would delete production data on the remote site. Data on an isCRR location is remote production data and must never be garbage-collected. Make it a general rule keyed on the location type rather than a clean-room special case, so it also holds for a plain lifecycle transition out of such a location. The rollback paths, which collect the freshly written local copy, are unaffected. This also underpins the replay-safety of duplicate copy actions: the first merge must not collect the remote source. Issue: BB-813
Not triggering the collection from the copy engine only covers one publisher. The GC service is the single point where data actually gets deleted, so guard there too: any deleteData action targeting an isCRR location is dropped, whoever published it. The service only has data to delete, and it has no way to delete data living on a remote location. Getting such an action is a bug upstream, hence the warning, but there is nothing that can be done about it beyond skipping it. Issue: BB-818
francoisferrand
force-pushed
the
improvement/BB-816-skip-non-localized-en
branch
from
September 11, 2026 14:10
bbd96de to
8123def
Compare
francoisferrand
force-pushed
the
improvement/BB-813
branch
from
September 11, 2026 14:14
777d6f9 to
f9f4df4
Compare
Contributor
Author
|
/approve |
Base automatically changed from
improvement/BB-816-skip-non-localized-en
to
development/9.6
September 11, 2026 14:29
Contributor
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
This pull request did not target the following hotfix branch(es) so they
Please check the status of the associated issue BB-813. Goodbye francoisferrand. The following options are set: approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Data living on an
isCRRlocation is production data owned by a remote site. We may read it (that is the whole point of localizing a clean room object), but we must never delete it. That is a general rule keyed on the location type, not a clean room conditional and not behind a config flag.It needs to hold at two independent points, one commit each.
BB-813 - do not trigger the GC. The copy engine reuses the lifecycle transition pipeline:
CopyLocationTaskcopies the data locally, thenLifecycleUpdateTransitionTaskmerges the new location into the metadata and publishes adeleteDataaction for the old one. Against anisCRRsource that would wipe the remote production copy, so unlike a transition, the localization merge leaves the from-location alone and publishes nothing. This also gives us replay safety for duplicate copy actions: the second merge supersedes the first and collects its copy, which is only safe because the first never touched the remote source.BB-818 - guard the GC service. The same rule applied where data actually gets deleted, whatever published the action. The service only has data to delete, and it has no way to delete data living on a remote location. Getting such an action is a bug upstream, hence the warning rather than silence, but there is nothing to do about it here beyond skipping it. No completion metric is emitted for a delete that did not happen.
Both check the location of the data being collected rather than the object storage class, because the two can disagree. If the object changes while the copy is in flight - the queue populator captures its
eTagandlastModified, and the merge re-reads metadata later - the copy is discarded and we collect the local data we just wrote. Metadata was never updated at that point, so the storage class still names theisCRRlocation while the data to collect is local. Keying on the storage class would skip that collection and leak the local copy, which nothing else would ever pick up since metadata still points at the remote data.Issue: BB-813
Issue: BB-818