Return warmup permits in case primary allocation fails #175
Conversation
…source with warming up pool in single reactive process
|
@k-tokarev Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@k-tokarev Thank you for signing the Contributor License Agreement! |
|
Sorry for imports rearranged in this pull request. My IntelliJ IDEA did it automatically and I don't know how to disable this for one single project. |
|
Hi @k-tokarev , Thanks a lot for reporting the issue #174, and for this PR ! |
reactor-pool/src/test/java/reactor/pool/AllocationFailureHandlingTest.java
Outdated
Show resolved
Hide resolved
I committed all proposed changes. Can you review new version of Pull Request? |
…e of added new test
| @@ -451,7 +451,7 @@ else if (sig.isOnError()) { | |||
| final int mergeConcurrency = Math.min(poolConfig.allocationStrategy().warmupParallelism(), toWarmup + 1); | |||
| Flux.range(1, toWarmup) | |||
| .map(i -> warmupMono(i, toWarmup, startWarmupIteration, allocator).doOnSuccess(__ -> drain())) | |||
| .startWith(primary.doOnSuccess(__ -> drain()).then()) | |||
| .startWith(primary.doOnSuccess(__ -> drain()).onErrorComplete().then()) | |||
There was a problem hiding this comment.
As was discussed yesterday, this solves the issue for now but can be further improved to decouple unused permit return from connection acquisition. Right now it is implicit what happens and a reader of the code might scratch their head for some time to understand why this is used.
|
@chemicL , thanks for the review, we'll try to refactor all this in the next version. @k-tokarev , thanks for the PR, which is merged. |
pderop
changed the title
violetagg
added
warn/regression
In order to support my issue (#174 (comment)) I made this pull request with test indicating the problem and my "naive" fix for it. Project builds successfully with this fix, so I hope it don't brake anything.