<div dir="ltr">The only thing I'd argue with here is the log level, Robert. Logstash on the gate doesn't index trace/debug, so info or above would be far more helpful, so that we can have a logstash query for the issue<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 25 February 2015 at 01:20, Robert Collins <span dir="ltr"><<a href="mailto:robertc@robertcollins.net" target="_blank">robertc@robertcollins.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 23 February 2015 at 13:54, Michael Bayer <<a href="mailto:mbayer@redhat.com">mbayer@redhat.com</a>> wrote:<br>
<br>
> Correct me if I'm wrong but the register_after_fork seems to apply only to<br>
> the higher level Process abstraction.   If someone calls os.fork(), as is<br>
> the case now, there's no hook to use.<br>
><br>
> Hence the solution I have in place right now, which is that Oslo.db *can*<br>
> detect a fork and adapt at the most basic level by checking for os.getpid()<br>
> and recreating the connection, no need for anyone to call engine.dispose()<br>
> anywhere. But that approach has been rejected.  Because the caller of the<br>
> library should be aware they're doing this.<br>
><br>
> If we can all read the whole thread here each time and be on the same page<br>
> about what is acceptable and what's not, that would help.<br>
<br>
</span>I've read the whole thread :).<br>
<br>
I don't agree with the rejection you received :(.<br>
<br>
Here are my principles in the design:<br>
 - oslo.db is meant to be a good [but opinionated] general purpose db<br>
library: it is by and for OpenStack, but it can only assume as givens<br>
those things which are guaranteed the same for all OpenStack projects,<br>
and which we can guarantee we don't want to change in future.<br>
Everything else it needs to do the usual thing of offering interfaces<br>
and extension points where its behaviour can be modified.<br>
 - failing closed is usually much much better than failing open. Other<br>
libraries and app code may do things oslo.db doesn't expect, and<br>
oslo.db failing in a hard to debug fashion is a huge timewaste for<br>
everyone involved.<br>
 - faults should be trapped as close to the moment that it happened as<br>
possible. That is, at the first sign.<br>
 - correctness is more important than aesthetics : ugly but doing the<br>
right thing is better than looking nice but breaking.<br>
 - where we want to improve things in a program in a way thats<br>
incompatible, we should consider a deprecation period.<br>
<br>
<br>
Concretely, I think we should do the following:<br>
 - in olso.db today, detect the fork and reopen the connection (so the<br>
users code works); and log a DEBUG/TRACE level message that this is a<br>
deprecated pattern and will be removed.<br>
 - follow that up with patches to all the projects to prevent this<br>
happening at all<br>
 - wait until we're no longer doing security fixes to any branch with<br>
the pre-fixed code<br>
 - at the next major release of oslo.db, change it from deprecated to<br>
hard failure<br>
<br>
That gives a graceful migration path and ensures safety.<br>
<br>
As to the potential for someone to deliberately:<br>
 - open an oslo.db connection<br>
 - fork<br>
 - expect it to work<br>
<br>
I say phoooey. Pre-forking patterns don't need this (it won't use the<br>
connect before work is handed off to the child). Privilege dropping<br>
patterns could potentially use this, but they are rare enough that<br>
they can explicitly close the connection and make a new one after the<br>
fork. In general anything related to fork is going to break and one<br>
should re-establish things after forking. The exceptions are<br>
sufficiently rare that I think we can defer adding apis to support<br>
them (e.g. a way to say 'ok, refresh your cache of the pid now') until<br>
someone actually wants that.<br>
<br>
-Rob<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
--<br>
Robert Collins <<a href="mailto:rbtcollins@hp.com">rbtcollins@hp.com</a>><br>
Distinguished Technologist<br>
HP Converged Cloud<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature">Duncan Thomas</div>
</div>