Repository navigation
Copy pulled data on the data processor of the location pulled from - #2933
francoisferrand wants to merge 4 commits into
Conversation
Hello francoisferrand,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| if (!locations[objMD.getDataStoreName()]?.isCRR) { | ||
| return { StorageType: this.destType, StorageClass: this.site }; | ||
| } | ||
| const { targetLocation } = objMD.getLocation()[0]; |
There was a problem hiding this comment.
This re-derives the target from location[0].targetLocation, which differs from what the populator resolved in _getPullReplicationTarget:
- empty object without location (explicitly allowed by the populator):
objMD.getLocation()isnull/[], so destructuring throws aTypeError(first hit in_checkObjectState, inside the waterfall). - invalid
targetLocation(populator falls back todefaultLocalLocation): data is written to a non-existent location, andStorageTypeisundefined.
Pass actionEntry and use the target the populator already carries in metrics.fromLocation when metrics.origin === PULL_REPLICATION, rather than inferring from object state.
| } | ||
| } | ||
| if (objMD.getDataStoreName() === actionEntry.getAttribute('toLocation')) { | ||
| if (objMD.getDataStoreName() === this._getDestStorage(objMD).StorageClass) { |
There was a problem hiding this comment.
Duplicate pull entries are expected (see populator doc). Once the first one completes, dataStoreName is the local target (not isCRR), so _getDestStorage falls back to this.site (the CRR location) and this check no longer skips: the task copies the data from the local location back to the CRR location, then publishes a result claiming it went to the target. Before this PR, toLocation === target made it skip.
Base the check on the action, e.g. for pull compare against metrics.fromLocation (or skip whenever the data is no longer on toLocation).
There was a problem hiding this comment.
Still applies with the reworked _setupDestination: on a replayed pull entry the data is already on the local target (not isCRR), so this.site stays the CRR location, _checkObjectState passes, and the task copies the data back to the CRR location, then publishes toLocation = the CRR location, which LifecycleUpdateTransitionTask writes into the metadata. For metrics.origin === PULL_REPLICATION with data no longer on a CRR location, return InvalidObjectState ("already pulled") from _setupDestination.
| } | ||
|
|
||
| _publishCopyLocationStatus(err, actionEntry, kafkaEntry, log) { | ||
| if (actionEntry.getAttribute('metrics.origin') === PULL_REPLICATION) { |
There was a problem hiding this comment.
Metrics now get the wrong labels for pull: processQueueEntry calls onReplicationProcessed(origin, fromLocation, this.site, ...), and this.site is now the CRR location rather than the target, so it no longer matches onReplicationQueued(PULL_REPLICATION, source, targetLocation) in the populator (the backlog never drains). Same for onLifecycleStarted, which reports the reversed toLocation. Use actionEntry.getAttribute('toLocation') (after this swap) instead of this.site, and the real target for onLifecycleStarted.
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 #2933 +/- ##
===================================================
- Coverage 82.04% 81.83% -0.21%
===================================================
Files 217 217
Lines 45409 45451 +42
===================================================
- Hits 37255 37196 -59
- Misses 8154 8255 +101
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| * @return {null|Error} error if there is no location to pull the data to | ||
| */ | ||
| _setupDestination(actionEntry, objMD, log) { | ||
| if (!locations[objMD.getDataStoreName()]?.isCRR) { |
There was a problem hiding this comment.
This retargets any copy of data that sits on a CRR location, not only pull replication. A lifecycle transition (toLocation: aws-loc) of an object still on dr-source is now written to targetLocation/us-east-1. _publishCopyLocationStatus keeps toLocation: aws-loc for non-pull origins, so the metadata names aws-loc while the data is elsewhere. Gate on the action origin, the same way the publish side does.
| if (!locations[objMD.getDataStoreName()]?.isCRR) { | |
| if (actionEntry.getAttribute('metrics.origin') !== PULL_REPLICATION) { |
Data processors only take the copies whose toLocation is their own site, and only the processor of the remote location can read the data to pull. So the populator now sends pull replications with toLocation set to the location pulled from (and the actual target in fromLocation): routing and the rest of the copy work as for any other copy. The copy task only needs to write the data to the location the object data points to rather than to its own site, and to restore the actual direction when publishing the result to the transition processor. Issue: BB-913
Rather than picking the destination in each write command, set the site and its type once the object metadata is known: a task handles a single action. The processed and started metrics then report the actual target. Issue: BB-913
The copy task now decides where pulled data goes, so resolve the target there, with the fallback the populator used to apply: objects without location, or naming a location which does not exist anymore, are pulled to the default local location. The populator just routes the action to the processor of the location pulled from. Issue: BB-913
f3e3ae7 to
077c7e5
Compare
| } | ||
| const sourceLocation = queueEntry.getDataStoreName(); | ||
| // for metrics only: the copy task resolves where the data actually goes | ||
| const { targetLocation } = locations?.[0] ?? {}; |
There was a problem hiding this comment.
The populator now uses the raw targetLocation, but CopyLocationTask may fall back to the default local location. When the target is unknown or missing, onReplicationQueued (and onLifecycleTriggered) get labelled with undefined/the deleted location, while onReplicationProcessed reports the fallback. That backlog series then never drains. Either label both sides with the same value (e.g. move the fallback into a shared helper used here for metrics only), or record the queued bytes in the task once the target is resolved.
Pull replication is tracked per location it pulls from, so the triggered and completed lifecycle metrics use that location like the started one, instead of the local location the data is copied to. Issue: BB-913
|
Abandoned in favor of #2876 |
Alternative to #2876 and #2931, keeping the change as local as possible.
Data processors only take the copies whose
toLocationis their own site, and only the processor of the remote location (e.g.dr-source) can read the data to pull. So rather than changing how messages are routed, the populator now sends pull replications "reversed":toLocationis the location pulled from, and the actual target goes infromLocation. Routing, credentials and reads then work as for any other copy.In
CopyLocationTask, two things differ for a pull:targetLocationof the location part), not to the processor's own site; this is detected from the object metadata (data on a CRR location), like the read side already doestoLocationandfromLocationare swapped back, soLifecycleUpdateTransitionTaskis unchangedIssue: BB-913