I believe sqlalchemy already tracks the session state, though I haven't seen anything (in the docs or code that I've touched) suggesting it knows how to be selective in issuing ROLLBACK. It would be a good feature, though I still think the sql_idle_timeout should be lowered ;)<div>
<br></div><div>-Deva<br><br><br><div class="gmail_quote">On Wed, Jul 11, 2012 at 1:43 PM, Mark Gius <span dir="ltr"><<a href="mailto:mark@markgius.com" target="_blank">mark@markgius.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span style="font-size:13px;font-family:arial,sans-serif">I would also love to see these changes applied.</span><div style="font-size:13px;font-family:arial,sans-serif">
<br></div><div style="font-size:13px;font-family:arial,sans-serif">With regards to the bugs around not issuing a commit or rollback, is it possible to have sqlachemy track whether or not a transaction starts and only issue a rollback when a session is handed back with an open transaction on it? Seems like a useful defensive measure.</div>
<span class="HOEnZb"><font color="#888888">
<div style="font-size:13px;font-family:arial,sans-serif"><br></div><div style="font-size:13px;font-family:arial,sans-serif">Mark</div></font></span><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote">
On Wed, Jul 11, 2012 at 12:46 PM, Jay Pipes <span dir="ltr"><<a href="mailto:jaypipes@gmail.com" target="_blank">jaypipes@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+1 to all your ideas below, Devananda.<br>
<div><div><br>
On 07/11/2012 01:33 PM, Devananda van der Veen wrote:<br>
> Hi all,<br>
><br>
> I've been taking a look at the way Nova uses its MySQL database. Having<br>
> done MySQL performance audits for years as a consultant, a few things<br>
> jumped out right away at me. First is the way that SQLAlchemy is<br>
> wrapping nearly every query in an unnecessary "ping check" and rollback,<br>
> eg.:<br>
><br>
> select 1;<br>
> select ... from ... where ...;<br>
> rollback;<br>
> select 1;<br>
> update ... where ...;<br>
> commit;<br>
> rollback;<br>
><br>
><br>
> You can find a complete sample here: <a href="http://paste.openstack.org/show/18731/" target="_blank">http://paste.openstack.org/show/18731/</a><br>
><br>
> I think I understand the reason for both the "select 1" and the<br>
> "rollback" statements. However, in the interest of performance for<br>
> large-scale deployments, I feel pretty strongly that they need to go away.<br>
><br>
> As I see it, there are three factors here.<br>
><br>
> (I) Most of the code in db/sqlalchemy/api.py abstracts a "unit of work"<br>
> to a very low level, generally a single database read or write, and does<br>
> not currently support transactions spanning multiple consistent writes.<br>
> This is why "select 1" and "rollback" appear around almost every query<br>
> -- most functions in api.py is checking out a session object, doing one<br>
> or two queries, and then releasing the session. This actually creates a<br>
> much larger issue about transactional atomicity for larger operations,<br>
> such as the open bug about network creation here, and is probably better<br>
> for another discussion.<br>
> <a href="https://bugs.launchpad.net/nova/+bug/755138" target="_blank">https://bugs.launchpad.net/nova/+bug/755138</a>.<br>
><br>
> (II) Connections are tested by the MySQLPingListener function, using<br>
> "SELECT 1" statements, every time a connection / session is checked out<br>
> of the pool. Since connections are usually OK, this adds overhead<br>
> unnecessarily. It would be more efficient to handle the errors when<br>
> connections aren't OK. I've opened a bug with a description of one<br>
> possible way to fix this issue. There are probably other viable<br>
> solutions as well.<br>
> <a href="https://bugs.launchpad.net/nova/+bug/1007027" target="_blank">https://bugs.launchpad.net/nova/+bug/1007027</a><br>
><br>
> My understanding is that this was implemented to prevent "Database has<br>
> gone away" errors from occurring every time that the database is<br>
> restarted, or when connections are closed by the database after being<br>
> idle for long periods. In my opinion, a better solution to these<br>
> problems is to:<br>
> * wrap queries in retry logic, which will catch disconnect errors and<br>
> attempt to reconnect to the database. I have a patch for this, if folks<br>
> are interested.<br>
> * set Nova's sql_idle_timeout to less than MySQL's wait_timeout, and set<br>
> them both to a reasonably short interval (eg, 1 minute), rather than the<br>
> default (which I think is 8 hours).<br>
><br>
> (III) Transaction state is reset when connections are returned to the<br>
> pool, even if no transaction was in progress, or the<br>
> transaction-in-progress already committed. This is completely wasteful,<br>
> and easy to disable.<br>
> <a href="https://bugs.launchpad.net/nova/+bug/1007038" target="_blank">https://bugs.launchpad.net/nova/+bug/1007038</a><br>
><br>
> Caveat here is that this reset-on-return functionality *is* useful when<br>
> code doesn't properly clean up its own transaction state. When I turned<br>
> it off, it exposed a few bugs, which I haven't tracked down yet.<br>
> Lowering the sql_idle_timeout will provide the same "solution" as the<br>
> current reset-on-return behavior in that it will prevent long-running<br>
> idle transactions that tie up resources unnecessarily.<br>
><br>
><br>
> In summary, I'd like to see Nova stop spamming the database with "SELECT<br>
> 1" and "ROLLBACK", and think this should be pretty easy to do. Testing<br>
> these two changes in my devstack seems to work fine, but I'd like to<br>
> know what others think before I propose a changeset with this kind of<br>
> potential impact.<br>
><br>
> Cheers,<br>
> Devananda<br>
><br>
><br>
</div></div>> _______________________________________________<br>
> Mailing list: <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
> Post to : <a href="mailto:openstack@lists.launchpad.net" target="_blank">openstack@lists.launchpad.net</a><br>
> Unsubscribe : <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
> More help : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
><br>
<br>
<br>
_______________________________________________<br>
Mailing list: <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
Post to : <a href="mailto:openstack@lists.launchpad.net" target="_blank">openstack@lists.launchpad.net</a><br>
Unsubscribe : <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
More help : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
</blockquote></div><br>
</div></div><br>_______________________________________________<br>
Mailing list: <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
Post to : <a href="mailto:openstack@lists.launchpad.net">openstack@lists.launchpad.net</a><br>
Unsubscribe : <a href="https://launchpad.net/~openstack" target="_blank">https://launchpad.net/~openstack</a><br>
More help : <a href="https://help.launchpad.net/ListHelp" target="_blank">https://help.launchpad.net/ListHelp</a><br>
<br></blockquote></div><br></div>