Skip to content

Count pending connects when considering max pool size #544

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
Merged
merged 1 commit into from
Mar 19, 2020
Merged

Count pending connects when considering max pool size #544

merged 1 commit into from
Mar 19, 2020

Conversation

2hdddg
Copy link
Contributor

@2hdddg 2hdddg commented Mar 19, 2020

Without this there could be a burst of connect and neogitations with the
server on connections just to be shut down immediately.

Count pending connects when considering max pool size
49e8906 
Without this there could be a burst of connect and neogitations with the
server on connections just to be shut down immediately.
@2hdddg 2hdddg requested a review from zhenlineo March 19, 2020 12:25
@2hdddg 2hdddg linked an issue Mar 19, 2020 that may be closed by this pull request
MaxConnectionPoolSize is not respected under heavy load #501
Closed
@2hdddg 2hdddg requested a review from jharris4 March 19, 2020 12:36
@jharris4
Copy link
Contributor

Hi @2hdddg, your changes look good to me! I assume that the test case you added was failing before you made these changes?

@2hdddg
Copy link
Contributor Author

2hdddg commented Mar 19, 2020

Hi @2hdddg, your changes look good to me! I assume that the test case you added was failing before you made these changes?

Yes it was. Also ran the test program mentioned in the linked issue. Verified by logging in the connection code, there were the correct number of connects.

@jharris4
Copy link
Contributor

Hi @2hdddg, your changes look good to me! I assume that the test case you added was failing before you made these changes?

Yes it was. Also ran the test program mentioned in the linked issue. Verified by logging in the connection code, there were the correct number of connects.

Nice! From a JS perspective this all looks good to me, I don't have any other feedback.

@zhenlineo zhenlineo merged commit f071896 into neo4j:4.0 Mar 19, 2020
zhenlineo
zhenlineo reviewed Mar 19, 2020
View reviewed changes
Copy link
Contributor

@zhenlineo zhenlineo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@2hdddg 2hdddg deleted the count-pending-connects branch March 20, 2020 06:13
@zhenlineo zhenlineo mentioned this pull request Mar 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zhenlineo zhenlineo zhenlineo left review comments

@jharris4 jharris4 jharris4 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MaxConnectionPoolSize is not respected under heavy load
3 participants
@2hdddg @jharris4 @zhenlineo