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

Set max pool size before core pool size #1458

Closed
wants to merge 1 commit into from

Conversation

icecreamhead
Copy link

As of JDK11 (tested on openjdk 11 & 12) an exception is thrown when attempting to set core pool size greater than max pool size

public void setCorePoolSize(int corePoolSize) {
        if (corePoolSize < 0 || maximumPoolSize < corePoolSize)
            throw new IllegalArgumentException();
        int delta = corePoolSize - this.corePoolSize;
        this.corePoolSize = corePoolSize;
        if (workerCountOf(ctl.get()) > corePoolSize)
            interruptIdleWorkers();
        else if (delta > 0) {
            // We don't really know how many new threads are "needed".
            // As a heuristic, prestart enough new workers (up to new
            // core size) to handle the current number of tasks in
            // queue, but stop if queue becomes empty while doing so.
            int k = Math.min(delta, workQueue.size());
            while (k-- > 0 && addWorker(null, true)) {
                if (workQueue.isEmpty())
                    break;
            }
        }
    }

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #1458 into dev will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##                dev    #1458   +/-   ##
=========================================
  Coverage     70.11%   70.11%           
  Complexity      561      561           
=========================================
  Files            26       26           
  Lines          2118     2118           
  Branches        296      296           
=========================================
  Hits           1485     1485           
  Misses          487      487           
  Partials        146      146           
Impacted Files Coverage Δ Complexity Δ
...c/main/java/com/zaxxer/hikari/pool/HikariPool.java 81.00% <0.00%> (ø) 59.00 <0.00> (ø)

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 30c63c6...bc28403. Read the comment docs.

@quaff
Copy link
Contributor

quaff commented Oct 10, 2019

Will this conflict with e8e9cdd ?

@icecreamhead
Copy link
Author

That commit doesn't make sense to me. It appears to do nothing other than introduce the bug that my commit fixes.

@johnou
Copy link
Contributor

johnou commented Oct 12, 2020

@icecreamhead please rebase.

@icecreamhead
Copy link
Author

@johnou done

@johnou
Copy link
Contributor

johnou commented Oct 13, 2020

@brettwooldridge ping

@icecreamhead
Copy link
Author

@brettwooldridge any chance of a review on this? Is this repo even maintained any more?

@johnou
Copy link
Contributor

johnou commented Jan 8, 2021

just noticed there is #1605 too which applies the fix in two places

@brettwooldridge
Copy link
Owner

Merged #1605

@icecreamhead icecreamhead deleted the patch-1 branch January 12, 2021 14:42
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.

None yet

4 participants