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

see #1804 Enforce 100K task cap on Schedulers.boundedElastic() #1900

Merged
merged 2 commits into from
Sep 20, 2019

Conversation

simonbasle
Copy link
Contributor

@simonbasle simonbasle commented Sep 20, 2019

Another late change, but if we introduce the boundedElastic() singleton version of the scheduler, we might as well make it truly bounded, including in the number of task submissions it can "absorb" during a spike. Used to be unbounded, now bounded to 100K tasks.

This is also more likely to uncover problematic scheduling of blocking tasks. Users switching from elastic() to boundedElastic() and seeing RejectedExecutionExceptions should challenge why their Reactor app schedules so many blocking tasks, and if still legitimate and necessary, use a newBoundedElastic instance that is better tuned to their workload.

@simonbasle simonbasle force-pushed the tuneBoundedElasticDefaultLimits branch from 13d269b to 262ea1d Compare September 20, 2019 08:29
@codecov-io
Copy link

codecov-io commented Sep 20, 2019

Codecov Report

Merging #1900 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1900      +/-   ##
============================================
+ Coverage     81.54%   81.61%   +0.06%     
- Complexity     3970     3971       +1     
============================================
  Files           373      373              
  Lines         30769    30769              
  Branches       5792     5792              
============================================
+ Hits          25092    25111      +19     
+ Misses         4078     4063      -15     
+ Partials       1599     1595       -4
Impacted Files Coverage Δ Complexity Δ
...c/main/java/reactor/core/scheduler/Schedulers.java 89.06% <100%> (ø) 89 <0> (-1) ⬇️
.../java/reactor/core/publisher/BlockingIterable.java 78.12% <0%> (-1.05%) 7% <0%> (ø)
...eactor/core/publisher/ParallelMergeSequential.java 80.31% <0%> (-1.04%) 7% <0%> (ø)
...java/reactor/core/scheduler/ExecutorScheduler.java 79.16% <0%> (-0.7%) 12% <0%> (ø)
...ain/java/reactor/core/publisher/FluxConcatMap.java 90% <0%> (+0.27%) 7% <0%> (ø) ⬇️
...c/main/java/reactor/core/publisher/FluxReplay.java 84.29% <0%> (+0.45%) 28% <0%> (+1%) ⬆️
...eactor/core/scheduler/BoundedElasticScheduler.java 83.72% <0%> (+0.58%) 39% <0%> (ø) ⬇️
.../java/reactor/core/scheduler/ElasticScheduler.java 85.84% <0%> (+1.76%) 26% <0%> (ø) ⬇️
...actor/core/publisher/FluxOnBackpressureLatest.java 83.5% <0%> (+3.09%) 2% <0%> (ø) ⬇️
...in/java/reactor/core/publisher/FluxWindowWhen.java 85.57% <0%> (+5.76%) 2% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f705674...a165a3c. Read the comment docs.

@simonbasle simonbasle marked this pull request as ready for review September 20, 2019 08:30
@simonbasle simonbasle force-pushed the tuneBoundedElasticDefaultLimits branch from 0df08ca to 10ad442 Compare September 20, 2019 10:09
@simonbasle simonbasle force-pushed the tuneBoundedElasticDefaultLimits branch from 10ad442 to a165a3c Compare September 20, 2019 11:27
@simonbasle simonbasle merged commit 32d2a29 into master Sep 20, 2019
@simonbasle simonbasle deleted the tuneBoundedElasticDefaultLimits branch September 20, 2019 12:21
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.

3 participants