Conversation
… finished ($still_running at false) before curl_multi_info_read. In that case, the spans could have been removed from the storage
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2847 +/- ##
============================================
+ Coverage 81.06% 81.09% +0.02%
+ Complexity 2516 2515 -1
============================================
Files 146 146
Lines 14653 14648 -5
============================================
Hits 11879 11879
+ Misses 2774 2769 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
iamluc
force-pushed
the
luc/fix-curl-integration-race-condition
branch
from
1133a8a to
c4c916f
Compare
bwoebi
left a comment
bwoebi
left a comment
There was a problem hiding this comment.
The difference is essentially that $saveSpans gets always set. I think that's right.
BenchmarksBenchmark execution time: 2024-09-12 12:24:53 Comparing candidate commit c4c916f in PR branch Found 1 performance improvements and 5 performance regressions! Performance is the same for 172 metrics, 0 unstable metrics. scenario:BM_TeaSapiSpindown
scenario:EmptyFileBench/benchEmptyFileBaseline-opcache
scenario:LaravelBench/benchLaravelBaseline-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching2
scenario:SamplingRuleMatchingBench/benchRegexMatching4
scenario:WordPressBench/benchWordPressOverhead
|
3 participants
Description
Spotted by the CI, though not frequent.
e.g. https://app.circleci.com/pipelines/github/DataDog/dd-trace-php/17462/workflows/f2a5d19c-4f05-4f57-aed9-f400f6677938/jobs/5150058
Fix a race condition where curl_multi_exec is called many times while finished ($still_running at false) before curl_multi_info_read.
In that case, the spans could have been removed from the storage.
+
Remove weird code using the last instance of a previous loop value ($requestSpan)Reviewer checklist