ap / DBIx-Connector

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

DEALLOCATE spam after ->disconnect #39

Closed johto closed 9 years ago

johto commented 9 years ago

Hi,

Currently, this line when running with DBD::Pg causes a monstrous DEALLOCATE spam to the server before disconnecting the session. PostgreSQL doesn't care if you disconnect with prepared statements open, so this is just completely unnecessary overhead and log spam. What are your thoughts on providing a way to disable the behaviour for those of us who aren't running one of the "some databases"?

johto commented 9 years ago

Well I've implemented one possible solution of this in #40; feel free to make fun of my attempt there.

theory commented 9 years ago

Why would your server be logging DEALLOCATE calls, unless you have on full statement logging?

johto commented 9 years ago

Why would your server be logging DEALLOCATE calls, unless you have on full statement logging?

You hit the nail on the head.

But I'm more worried about the hundreds of roundtrips it has to do to the server while a client is waiting on the other end.

theory commented 9 years ago

It can't be much compared to the 100s of round-trips used to allocate those prepared statements, let alone execute them.

You can pass pg_server_prepare => 0 to prevent the allocation of prepared statements on the server if you don't need them.

johto commented 9 years ago

It can't be much compared to the 100s of round-trips used to allocate those prepared statements, let alone execute them.

One at a time, over time. Not comparable.

theory commented 9 years ago

Then maybe you need to use prepare() instead of prepare_cached()?

johto commented 9 years ago

Then maybe you need to use prepare() instead of prepare_cached()?

There are still tons of executions of the same prepared statements, so no, that doesn't seem like a good workaround. And no, I can't tell which ones they're going to be beforehand (with a few of exceptions).

theory commented 9 years ago

Well, I wonder, then, if DBD::Pg should be updated to not send DEALLOCATE statements when cached statements are removed from memory. Or maybe not at all. DBIx::Connector does not fee like the right place for this functionality to me.

johto commented 9 years ago

Well, I wonder, then, if DBD::Pg should be updated to not send DEALLOCATE statements when cached statements are removed from memory. Or maybe not at all.

That would leak prepared statements over the lifetime of the connection, which doesn't sound like a good idea.

DBIx::Connector does not fee like the right place for this functionality to me.

Huh? DBIx::Connector is where the functionality is coming from right now. I'm trying to provide a way to make it go away, since it's having a negative effect on my application.

theory commented 9 years ago

Okay, after a bit of discussion on #postgresql, I think the thing to do is to actually deprecate and then remove this functionality altogether. If some drivers spew a bunch of warnings from cached kids on disconnect, that is either a bug in those drivers, or else something the end user needs to hear about.

johto commented 9 years ago

Thanks.

theory commented 9 years ago

@johto No problem. I'll wait a few days to see if the DBIx::Class folks find some other reason to keep this code, and if not, I'll release. Time for a new version, anyway.

ribasushi commented 9 years ago

@theory Sorry for the late reply - was too busy traveling. Removing this code from DBIx::Connector seems sane, I am still thinking how to better handle this within DBIx::Class.

theory commented 9 years ago

Oh, thanks @ribasushi. Will push it out this week.