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.

Sign up for GitHub

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

Jump to bottom

Dispose picked worker when BoundedElasticScheduler rejects task #3183

Merged

simonbasle merged 2 commits into reactor:3.4.x from Fearsin:3182-dispose-picked-after-rejection

Sep 20, 2022

Contributor

Fearsin commented Sep 6, 2022

If BoundedState picked and rejected, BoundedState not decrement markCount. After RejectedExecutionException BoundedState never move to idle.

Fixes #3182.

Signed-off-by: Stanislav Kotik [email protected]
Co-authored-by: Stanislav Kotik [email protected]

Fearsin requested a review from a team as a code owner

September 6, 2022 21:29

chemicL requested changes

View reviewed changes

Member

chemicL left a comment

Thank you for the contribution. Please consider targeting 3.4.x branch as these changes are also applicable to the GA codebase. I left a few language related suggestions and a few pointers to improve the test.

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

Member

chemicL commented Sep 14, 2022 •

edited

Loading

Also, it might not be codified, but seems we're using the style of

// comment

vs

//comment

as the first one looks cleaner.

chemicL reviewed

View reviewed changes

reactor-core/src/main/java/reactor/core/scheduler/BoundedElasticScheduler.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/main/java/reactor/core/scheduler/BoundedElasticScheduler.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/main/java/reactor/core/scheduler/BoundedElasticScheduler.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

chemicL reviewed

View reviewed changes

reactor-core/src/test/java/reactor/core/scheduler/BoundedElasticSchedulerTest.java Outdated Show resolved Hide resolved

OlegDokuka approved these changes

View reviewed changes

Contributor

OlegDokuka commented Sep 19, 2022

@Fearsin LGTM with minor comments from @chemicL.

OlegDokuka added this to the 3.4.24 milestone

OlegDokuka added the type/bug label

Fearsin changed the base branch from main to 3.4.x

September 19, 2022 18:05

Fearsin changed the base branch from 3.4.x to main

September 19, 2022 18:07

Fearsin added a commit to Fearsin/reactor-core-fork that referenced this pull request


          Dispose picked if throw RejectedExecutionException (reactor#3182)

22ebde2

Review fixes. Requested changes from @chemicL

Fixes reactor#3182 reactor#3183.

Signed-off-by: Stanislav Kotik <[email protected]>
Co-authored-by: Stanislav Kotik <[email protected]>

Fearsin changed the base branch from main to 3.4.x

September 20, 2022 10:01

Kuznetsov Stanislav and others added 2 commits

September 20, 2022 13:02


          Add dispose picked BoundedState if throw RejectedExecutionException (r…

28aca72

…eactor#3182)

If BoundedState picked and rejected, BoundedState not decrement markCount.
After RejectedExecutionException BoundedState never move to idle.

Fixes reactor#3182.

Signed-off-by: Stanislav Kotik <[email protected]>
Co-authored-by: Stanislav Kotik <[email protected]>


          Dispose picked if throw RejectedExecutionException (reactor#3182)

0ab2655

Review fixes. Requested changes from @chemicL

Fixes reactor#3182 reactor#3183.

Signed-off-by: Stanislav Kotik <[email protected]>
Co-authored-by: Stanislav Kotik <[email protected]>

Fearsin force-pushed the 3182-dispose-picked-after-rejection branch from 22ebde2 to 0ab2655 Compare

September 20, 2022 10:02

Fearsin requested review from chemicL and OlegDokuka and removed request for chemicL and OlegDokuka

September 20, 2022 10:05

OlegDokuka approved these changes

View reviewed changes

chemicL approved these changes

View reviewed changes

Member

chemicL left a comment

Thanks for addressing the comments. LGTM 🚢

simonbasle approved these changes

View reviewed changes

Contributor

simonbasle left a comment

thanks for the contribution (and good job on the retargetting to 3.4.x), LGTM 👍

simonbasle changed the title ~~Dispose picked if throw RejectedExecutionException #3182~~ Dispose picked worker when BoundedElasticScheduler rejects task

simonbasle merged commit f2ffd70 into reactor:3.4.x

reactorbot commented Sep 20, 2022

@simonbasle this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to main 🙇

simonbasle added a commit that referenced this pull request


          Merge #3183 into 3.5.0-RC1

f3242f4

simonbasle modified the milestones: 3.4.24, 3.5.0-M7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels