Open Stack

On 23 February 2015 at 13:54, Michael Bayer <mbayer at redhat.com> wrote:

> Correct me if I'm wrong but the register_after_fork seems to apply only to
> the higher level Process abstraction.   If someone calls os.fork(), as is
> the case now, there's no hook to use.
>
> Hence the solution I have in place right now, which is that Oslo.db *can*
> detect a fork and adapt at the most basic level by checking for os.getpid()
> and recreating the connection, no need for anyone to call engine.dispose()
> anywhere. But that approach has been rejected.  Because the caller of the
> library should be aware they're doing this.
>
> If we can all read the whole thread here each time and be on the same page
> about what is acceptable and what's not, that would help.

I've read the whole thread :).

I don't agree with the rejection you received :(.

Here are my principles in the design:
 - oslo.db is meant to be a good [but opinionated] general purpose db
library: it is by and for OpenStack, but it can only assume as givens
those things which are guaranteed the same for all OpenStack projects,
and which we can guarantee we don't want to change in future.
Everything else it needs to do the usual thing of offering interfaces
and extension points where its behaviour can be modified.
 - failing closed is usually much much better than failing open. Other
libraries and app code may do things oslo.db doesn't expect, and
oslo.db failing in a hard to debug fashion is a huge timewaste for
everyone involved.
 - faults should be trapped as close to the moment that it happened as
possible. That is, at the first sign.
 - correctness is more important than aesthetics : ugly but doing the
right thing is better than looking nice but breaking.
 - where we want to improve things in a program in a way thats
incompatible, we should consider a deprecation period.


Concretely, I think we should do the following:
 - in olso.db today, detect the fork and reopen the connection (so the
users code works); and log a DEBUG/TRACE level message that this is a
deprecated pattern and will be removed.
 - follow that up with patches to all the projects to prevent this
happening at all
 - wait until we're no longer doing security fixes to any branch with
the pre-fixed code
 - at the next major release of oslo.db, change it from deprecated to
hard failure

That gives a graceful migration path and ensures safety.

As to the potential for someone to deliberately:
 - open an oslo.db connection
 - fork
 - expect it to work

I say phoooey. Pre-forking patterns don't need this (it won't use the
connect before work is handed off to the child). Privilege dropping
patterns could potentially use this, but they are rare enough that
they can explicitly close the connection and make a new one after the
fork. In general anything related to fork is going to break and one
should re-establish things after forking. The exceptions are
sufficiently rare that I think we can defer adding apis to support
them (e.g. a way to say 'ok, refresh your cache of the pid now') until
someone actually wants that.

-Rob


-- 
Robert Collins <rbtcollins at hp.com>
Distinguished Technologist
HP Converged Cloud