-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Use LIFO for returning/retrieving connections from the pool #2085
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add this as a configurable option. A pool representing a read slave through a tcp load balancer works better with rr for instance, so there are likely folks who are relying on the current behavior. Both behaviors are valid, so it makes to add a configuration to choose.
Will do 👍 |
As for why it was initially implemented as rr it was before my time so I have no idea :) |
The implementation and test itself look good, though 👍 |
The test failures are a little funny, but it looks like I may have broken the |
Must have just been a randomly failing test. Should be good to go now 🙆 |
#1749 can probably be closed off if this pr gets merged. |
Gentle bump on this 😃. Had a chance to take a look over it @dougwilson ? |
Not since my initial look as I have been away. I will get back soon and take a look at the new changes |
All good 👍 Thanks for the message. |
946727b
to
37fbbdd
Compare
Did you get a chance to take a look over it? @dougwilson |
The current way this works (cycling through all of the connections) means that you can't make use of mysql's
wait_timeout
setting for closing inactive connections. Once a connection is created it will be persisted forever unless you have very low levels of activity.Was there a reason to cycle round-robin through the connections when this was initially implemented?