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

Applying settings to database() at runtime does not reuse database connections #75

Open
yahermann opened this issue Nov 20, 2015 · 9 comments

Comments

@yahermann
Copy link

Per documentation, using database() in this way:

my $dbh = database({ driver => 'SQLite', database => $filename });

causes a brand new connection to be created each time, and old connections don't get disconnected, so the max gets hits quickly.

The problem appears to be in Database.pm, lines 52-54:

if (ref $arg eq 'HASH') {
    $handle_key = $arg;
    $conn_details = _merge_settings($arg, $settings, $logger);
 } else {

Even though $arg may be the same hash, i.e., { database => "mydb", username="myuser" }, unless special precautions are taken, generating it during runtime creates a different anonymous hashref each time, therefore when used as a key, Perl treats them each as different keys. Since they're different keys, a new connection gets created with each call, and old connections/handles are not reused.

@ambs
Copy link
Collaborator

ambs commented Nov 21, 2015

Not sure if there is a way to create an unique hashref.
An option would be to serialize (sorting keys) and create md5 or something as key.
Other option would be to add a name => foo option, and reuse when name is the same.

@bigpresh , thoughts?

@yahermann
Copy link
Author

For anyone that runs into this problem, I kludged my way around this issue by setting the database definition hash globally:

use strict;
use Dancer ':syntax';
use Dancer::Plugin::Database;
....
my $db = { database => 'mydb', username => 'myuser', ... };
....
return -1;

and then anywhere a database handle is needed,

my $dbh = database($db);

The key is to avoid using this feature as shown in the documentation; otherwise, database handles will not be reused and you will quickly hit your max_connections threshold.

@ambs
Copy link
Collaborator

ambs commented Dec 15, 2015

Nice solution, @yahermann. Thanks for sharing!

@ambs
Copy link
Collaborator

ambs commented Dec 15, 2015

Let me reopen the ticket. We need to either correct the behavior or to document your approach and warn users.

@ambs ambs reopened this Dec 15, 2015
@yahermann
Copy link
Author

Fair enough :-)

@bigpresh
Copy link
Owner

Hmm, sorry I managed to miss this one. Yes, agreed, we ought to work out a fix - serialising the hashref contents to generate a key sounds like the best option which would DWYM.

@yahermann
Copy link
Author

I think serializing the hashref would work to generate a unique scalar key, as long as the hashref keys AND values AND subkeys/subvalues recursively (e.g. dbi_params => { ... } ) are all part of the scalar key, and sorted by each key/subkey within the hashref.

There must be a Perl module that does this in a reliable and consistent way (and hopefully already existing in the dependency chain). Otherwise you would DWYM intermittently, which is much worse!

Another approach, and one that I think I prefer: Require the user to manually specify a scalar key when using this option, explicitly for the purpose of reusing database handles.

@yahermann
Copy link
Author

I ran into another ugly problem using database() with a hashref, that is contrary to the documentation.

It appears the connection_check_threshold, which according to the documentation is supposed to default to 30 (seconds) if not specified, is actually not set at all when using database() with a hashref. It must be specified in the passed hashref. Failing to do so results in eventual and consistent 'mysql server has gone away' errors unless the database is used more frequently than whatever is specified in mysql wait_timeout (which defaults to 28,800 seconds, or 8 hours).

@bigpresh
Copy link
Owner

I finally had a chance to hack on this, and got part-way, but one fun problem: the settings may contain coderefs, e.g. HandleError, whatever serialisation form is used needs to understand that. It will probably have to just ignore coderefs, so there's the risk that, if all other arguments are the same but the coderef is different, you still get the cached handle; that sounds reasonable enough if documented. I'm going to head to bed in a moment, so I'll poke at this again as soon as time permits.

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

No branches or pull requests

3 participants