Repository navigation
ARSN-652: Fix leak in DataWrapper on KMS or stream error - #2715
Conversation
Hello bourgoismickael,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/8.4 #2715 +/- ##
===================================================
+ Coverage 74.44% 74.72% +0.27%
===================================================
Files 229 229
Lines 18566 18582 +16
Branches 3811 3847 +36
===================================================
+ Hits 13822 13885 +63
+ Misses 4739 4692 -47
Partials 5 5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BourgoisMickael
changed the title
3c5984c to
f3c007c
Compare
- destroy the get stream on get with KMS error - destroy the get stream on copy (for object and part) on both KMS or put error
f3c007c to
fe7a447
Compare
anurag4DSB
left a comment
anurag4DSB
left a comment
There was a problem hiding this comment.
I have a question about sequence of events we want to happen to not leave orphan threads. May be thats intentional
`.pipe()` only forwards data, backpressure and end. `pipeline` forwards errors and destroy streams for cleanup and prevent socket leak. Otherwise some socket might stay open for a while until garbage collection if any issue arise anywhere in the pipeline. Note: for _put value stream and error downstream is not propagated this is by design to let the V4Transform drain the incoming HTTP upload stream
fe7a447 to
9ff0079
Compare
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 |
|
/create_integration_branches |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: git fetch
git checkout -B w/8.5/improvement/ARSN-652-leak-kms origin/development/8.5
git merge origin/improvement/ARSN-652-leak-kms
# <intense conflict resolution>
git commit
git push -u origin w/8.5/improvement/ARSN-652-leak-kmsThe following options are set: create_integration_branches |
ConflictA conflict has been raised during the creation of I have not created the integration branch. Here are the steps to resolve this conflict: git fetch
git checkout -B w/8.6/improvement/ARSN-652-leak-kms origin/development/8.6
git merge origin/w/8.5/improvement/ARSN-652-leak-kms
# <intense conflict resolution>
git commit
git push -u origin w/8.6/improvement/ARSN-652-leak-kmsThe following options are set: create_integration_branches |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following options are set: create_integration_branches |
|
/approve |
|
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 ARSN-652. Goodbye bourgoismickael. The following options are set: approve, create_integration_branches |
4 participants
1st commit for prettier, otherwise changes are small
Destroy the data stream on backend error
on both KMS or put error
Replace pipe with pipeline for cleanup
.pipe()only forwards data, backpressure and end.pipelineforwards errors and destroy streams for cleanup and prevent socket leak.Otherwise some socket might stay open for a while until garbage collection
if any issue arise anywhere in the pipeline.
Note: for _put value stream and error downstream is not propagated
this is by design to let the V4Transform drain the incoming HTTP upload stream
Tests pass in