Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RUM-3670 use backpressured executors #1939

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

xgouchet
Copy link
Member

What does this PR do?

Apply the Backpressure strategy and blocking queue to all ExecutorServices.

Additional Notes

Note

We are still using our legacy LoggingScheduledThreadPoolExecutor for scheduled tasks (e.g.: uploader, vitals tracking, …). Those will only have one our two tasks in their queue, so it's not as urgent to use the backpressure strategy there. A task (RUM-3704) has been created to keep track of this.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@xgouchet xgouchet requested review from a team as code owners March 26, 2024 13:40
@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2024

Codecov Report

Merging #1939 (19454fc) into release/2.7.0 (c9243d1) will decrease coverage by 0.25%.
The diff coverage is 50.60%.

Additional details and impacted files
@@                Coverage Diff                @@
##           release/2.7.0    #1939      +/-   ##
=================================================
- Coverage          83.37%   83.13%   -0.25%     
=================================================
  Files                485      484       -1     
  Lines              17497    17529      +32     
  Branches            2598     2591       -7     
=================================================
- Hits               14588    14571      -17     
- Misses              2203     2256      +53     
+ Partials             706      702       -4     
Files Coverage Δ
.../datadog/android/core/internal/thread/ThreadExt.kt 60.47% <ø> (ø)
...rum/src/main/kotlin/com/datadog/android/rum/Rum.kt 91.84% <100.00%> (+0.35%) ⬆️
...lin/com/datadog/android/rum/internal/RumFeature.kt 92.99% <100.00%> (+0.30%) ⬆️
.../android/rum/internal/monitor/DatadogRumMonitor.kt 87.26% <100.00%> (+1.59%) ⬆️
...nal/tracking/AndroidXFragmentLifecycleCallbacks.kt 94.29% <100.00%> (-0.16%) ⬇️
...nternal/tracking/OreoFragmentLifecycleCallbacks.kt 89.47% <100.00%> (-0.27%) ⬇️
.../rum/tracking/ActivityLifecycleTrackingStrategy.kt 86.96% <100.00%> (+4.73%) ⬆️
...droid/rum/tracking/ActivityViewTrackingStrategy.kt 90.00% <100.00%> (+2.90%) ⬆️
...ain/kotlin/com/datadog/android/core/DatadogCore.kt 83.85% <77.78%> (-0.64%) ⬇️
...n/com/datadog/android/core/internal/CoreFeature.kt 86.69% <86.36%> (-0.48%) ⬇️
... and 2 more

... and 28 files with indirect coverage changes

@xgouchet xgouchet changed the title Xgouchet/rum 3670/backpressured executors RUM-3670 use backpressured executors Mar 26, 2024
Base automatically changed from xgouchet/RUM-3670/backpressure_strategy to release/2.7.0 March 26, 2024 16:27
@xgouchet xgouchet force-pushed the xgouchet/RUM-3670/backpressured_executors branch from 0aefb00 to b1b4ec6 Compare March 26, 2024 16:28
Copy link
Member

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I covered so far only production files in a quick look

@xgouchet xgouchet requested review from mariusc83 and 0xnm March 26, 2024 18:06
Copy link
Member

@0xnm 0xnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm. I left some comments/suggestions, but they are not blocking.

Copy link
Member

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM But I would really like us to think about what I added there as a opened question.

@xgouchet xgouchet merged commit fd490f2 into release/2.7.0 Mar 27, 2024
23 checks passed
@xgouchet xgouchet deleted the xgouchet/RUM-3670/backpressured_executors branch March 27, 2024 09:24
@xgouchet xgouchet added this to the 2.7.x milestone Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants