ap / DBIx-Connector

Fast, safe DBI connection and transaction management
https://metacpan.org/release/DBIx-Connector
40 stars 14 forks source link

proposal: ->dbh() call without pinging database #44

Closed bjakubski closed 8 years ago

bjakubski commented 8 years ago

I'd like to ask if a feature I find useful makes sense in DBIx::Connector.

At $work we have large codebase using our own "dbh manager". That manager has some generic features, that correspond to DBIx::Connector and some highly specific to our project. I wanted to use DBIx::Connector in our manager to 1. reduce amount of current code (as generic features would be provided by DBIx::Connector) 2. use some features not present in our manager, like ->run() without implementing them on our own. Problem is that our most prevalent API for dbh is simple global function for getting database handler (say "foo_dbh"). As it was known that it that function is quick it is used a lot in our code. When I substituted our old code inside our "dbh manager" with DBIx::Connector I found out that while everything works as intended things got sometimes noticeably slower. I traced this to dbh ping being perfomed on every $connector->dbh(). Our old function performed mostly the same operations as DBIx::Connector's equivalent with the exception of this ping...

Our code mostly assumes long idle timeouts (and no disconnects) on our handles, so it made sense to avoid ping. When long running things need to access DB after potential long inactivity on handler we perform explicit ping operation (via our manager). This is... not very nice and I hope to expose DBIx::Connector run() etc. methods via our manager to improve situation. But reducing dbh() calls is now simply infeasible for us.

I'd love to have something in line of

my $dbh = $connector->dbh(mode => 'no_ping')

which would solve our problem (as our current code is to be held responsible to ensure that dbh is truly "live")

I understand that this is quite specific and rare case and while functionality itself should be trivial to implement and do not cause any compatibility issues it may not fit DBIx::Connector's bigger picture (also even small features sooner or later will cause some burden to maintainers).

That is why I'm just asking if this is something which might get included in DBIx::Connector (assuming code with tests is provided)

theory commented 8 years ago

I don't think so. This is what the run(), txn(), and svp() methods are for: They handle the ping so you don't have to. The dbh() method is for compatibility, really. If you just want the database handle without doing all the checks, just call _dbh(), instead. But maybe wean yourself off of it over time.

bjakubski commented 8 years ago

Thanks for your answer. I guess my proposal was really to make _dbh public interface (although as an option that you pass to dbh()). I was afraid this will not be an option and I understand your rationale. I'll consider using _dbh in our code.

Thanks again!