-
-
Couldn't load subscription status.
- Fork 460
Avoid ExecutorService for DefaultCompositePerformanceCollector timeout #4841
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
Avoid ExecutorService for DefaultCompositePerformanceCollector timeout #4841
Conversation
…ollection for timeout on data collection rather than a scheduled background job
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 382.73 ms | 435.41 ms | 52.68 ms |
| 96449e8 | 361.30 ms | 423.39 ms | 62.09 ms |
| ee747ae | 374.71 ms | 455.18 ms | 80.47 ms |
| 3699cd5 | 423.60 ms | 495.52 ms | 71.92 ms |
| b3d8889 | 371.84 ms | 447.49 ms | 75.65 ms |
| 27d7cf8 | 369.82 ms | 422.62 ms | 52.80 ms |
| d364ace | 411.72 ms | 430.81 ms | 19.10 ms |
| 7314dbe | 437.83 ms | 505.64 ms | 67.81 ms |
| ce0a49e | 532.00 ms | 609.96 ms | 77.96 ms |
| 3998a95 | 415.94 ms | 478.54 ms | 62.60 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 96449e8 | 1.58 MiB | 2.11 MiB | 539.35 KiB |
| ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
| 3699cd5 | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| b3d8889 | 1.58 MiB | 2.10 MiB | 535.07 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| d364ace | 1.58 MiB | 2.11 MiB | 539.75 KiB |
| 7314dbe | 1.58 MiB | 2.10 MiB | 533.45 KiB |
| ce0a49e | 1.58 MiB | 2.10 MiB | 532.94 KiB |
| 3998a95 | 1.58 MiB | 2.10 MiB | 532.96 KiB |
Previous results on branch: stefanosiano/fix/performance-data-collection-timeout
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 77933c2 | 312.19 ms | 353.16 ms | 40.97 ms |
| f8c8b31 | 335.17 ms | 436.20 ms | 101.03 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 77933c2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| f8c8b31 | 1.58 MiB | 2.12 MiB | 551.20 KiB |
sentry/src/main/java/io/sentry/DefaultCompositePerformanceCollector.java
Outdated
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.
there's one comment I'm not sure about
…-timeout # Conflicts: # CHANGELOG.md
…a-collection-timeout' into stefanosiano/fix/performance-data-collection-timeout
|
@romtsn last check and approval? 😄 |
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!
📜 Description
DefaultCompositePerformanceCollector now stops the performance data collection for timeout on data collection rather than a scheduled background job
💡 Motivation and Context
Every time a transaction run, the SDK starts collecting performance data (cpu and ram usage) and schedules a timeout job after 30 seconds to stop this data collection.
Since v
8.19.0we have a limit on the queue of our background executor of 271 scheduled jobs. Any other scheduled job is ignored.This means that if a transaction runs forever and the background thread queue is full, its performance data will be collected forever, leading eventually to an OOM.
More in this doc
💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps