-
Notifications
You must be signed in to change notification settings - Fork 61
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
RUM-3670 use backpressured executors #1939
Conversation
Codecov Report
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
|
0aefb00
to
b1b4ec6
Compare
There was a problem hiding this 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
dd-sdk-android-core/src/main/kotlin/com/datadog/android/api/feature/FeatureSdkCore.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt
Show resolved
Hide resolved
...roid-core/src/main/kotlin/com/datadog/android/core/thread/ScheduledExecutorServiceFactory.kt
Outdated
Show resolved
Hide resolved
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt
Show resolved
Hide resolved
features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt
Show resolved
Hide resolved
There was a problem hiding this 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.
dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/CoreFeature.kt
Show resolved
Hide resolved
...c/test/kotlin/com/datadog/android/core/internal/thread/AbstractLoggingExecutorServiceTest.kt
Show resolved
Hide resolved
features/dd-sdk-android-rum/src/main/kotlin/com/datadog/android/rum/internal/RumFeature.kt
Show resolved
Hide resolved
There was a problem hiding this 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.
What does this PR do?
Apply the Backpressure strategy and blocking queue to all
ExecutorService
s.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)