[Twisted-Python] Consistent interfaces to asynchronous partially-available services using Deferreds and state machines (was Re: Another approach to allowing __init__ to work with Deferreds)

[Terry Jones]

Hi Glyph

>>>>> "glyph" == glyph  <glyph at divmod.com> writes:
> On 11 May, 04:19 pm, terry.jones at gmail.com wrote:
> 
> >I posted to this list back in Nov 2008 with subject:
> >A Python metaclass for Twisted allowing __init__ to return a Deferred
> 
> Let me try rephrasing your use-case here, for two reasons: one, I want to
> make sure I fully understand it, and two, I feel like the language this is
> couched in (hacks about __init__ and Deferreds and
> metaclass/mixin/decorator [ab]use) are detracting from the real core
> use-case here.

OK, fair enough. I'm happy to work upwards from my concrete problems to a
solution to a more general problem.

> You have a utility object which you want to create immediately and make
> immediately available to various pieces of calling code.  However, an
> instance of this class represents a shared interface to an external,
> asynchronous resource, to which you must establish a connection, and so
> you don't immediately have a connection when the class is created.

Yes, though I don't know why you use the word "shared". Perhaps because a
created instance might be passed to several other pieces of code that all
use it?

> However, you want to contain all this complexity behind a nice facade,
> and tell all the callers "just call these (Deferred-returning) methods
> and you will get sensible results no matter what state the connection is
> in".

Yes.

And as I mentioned in my later mail to Drew, a nice property of keeping the
complexity behind the interface is that when a vanilla class that is being
used in the manner you describe, is changed from being reliably in a single
state to having multiple states, the caller is not aware of that and does
not have to change how it instantiates or uses instances of the class.

> If my assessment of your use-case is flawed, please say so

Nope, that's about perfect.

> To skip ahead to the end: the answer is that you want a state-machine.
> And it is quite sad to me that Twisted doesn't have a nice, standard,
> full-featured state-machine class that we use for everything like this,
> because members of the Twisted team have implemented at least half a
> dozen of these, probably a lot more, in various applications.  I am like
> 90% sure that there's a ticket in the tracker for this, but I couldn't
> find it by searching around a bit.  I hope exarkun or jml or radix will
> have a better memory of this than I do.

I'll be interested to hear.

I've omitted a big chunk of your reply here - that I agree with and which
recaps what went down in the original thread and comments on Drew's
suggestion.

> But, although I still think this is generally good practice, it doesn't
> solve the underlying problem I think you're really getting at:
> consistency and convenience in the face of Deferred-ness.

I still like your approach (using a class method to hand you a fully
initialized instance), but didn't find it appropriate for my situation. The
main problem was that I was writing code in an __init__ method of a class
that was already in use by other code, including being called from the
__init__ method of other classes. Your solution is fine if you're in a
context where you can properly deal with deferreds. If you're not (e.g.,
you're in an __init__ method) then calling something that creates you an
instance of another class via a deferred just leaves you with the same
problem. I hope that's making sense.

Is the underlying problem "consistency and convenience in the face of
Deferred-ness"? You could look at it that way (and I'm happy to). My
__init__ case seems to be well summarized by my original comment:

    this is a general problem of the synchronous world (in which __init__
    is supposed to prepare a fully-fledged class instance and cannot return
    a deferred) meeting the asynchronous world in which we would like to
    (and must) use deferreds.

If Python allowed me to return a deferred from __init__, my problem would
vanish.  That's not going to happen though, I know :-)

Yours would remain though, and as you say, it's more general.

> Applications have to handle Deferreds from the connection's methods
> anyway, and there's no reason to force them to all have code to handle at
> least two (one for the connection, one for the actual application-level
> message), where one would do fine.

I'm not 100% sure that I follow this, but I think so.

> >Anyway.... fast forward 6 months and I've hit the same problem again.
> >It's with existing code, in which I would like an __init__ to call
> >something that (now, due to changes elsewhere) returns a deferred. So I
> >started thinking again, and came up with a much cleaner way to do the
> >alternate approach via a class mixin:
> 
> I think I like this a bit better than your earlier approaches.  It's
> automatic, its semantics are pretty clear, and it doesn't require any abuse
> of __init__'s implicit contract; your instance *is* in a fully valid state
> when it's created, it's just a different state than the state that it's in
> later.  However, you can still call all the same methods and get the same
> results.

Yes, those are the advantages. And the different state will, in most use
cases (I claim), be quite fleeting. It's that short-term not-quite-ready
window that the temporary state accounts for. And if the not-quite-ready
happens to not be short, then providing functionality like this (not
necessarily my implementation) is even more important.

I mention all this, for clarity, not for you - I know you already know -
but for others who might be reading along now or later.

> It still has one major flaw given your earlier example of a database
> connection (as I described above): it doesn't handle errors very well.

Right.

> In particular - and this is why you really need a state machine - it
> doesn't handle the case where errors start happening *later*.

OK, more on this below.

> It's also got a few implementation issues that you might not be aware of
> though - and you seem to appreciate a lot of detail in these responses,
> so I'll just look at it line by line, code-review style.

Yes, that's great, and thanks.

> I apologize in advance if this sounds like I'm being hypercritical - I
> realize you may have omitted certain details to keep this brief for
> discussion and so may have been aware of most of these problems.  Again,
> even if you fully understood all of these details I am sure there are
> many readers who didn't though :)

Some I'm aware of and skipped, others not.  I never know how much detail to
provide / go into, or if I'm bugging people on the list, etc.

> >    from twisted.internet import defer
> >
> >    class deferredInitMixin(object):
> >        def wrap(self, d, *wrappedMethods):
> 
> Just as a point of convenience, I would have automatically determined this
> list of method names by using a decorator or something.  Having it as a
> static list in the method invocation seems to me like it would be very easy
> to forget to add or remove a method from the list, and it would make diffs
> that touched a user of this class always have two hunks for adding a
> method; one at the method definition site, one at the call to wrap().

I started out trying to write this using decorators. But I didn't really
see how to do it. I was using two - one for __init__ and one for the
wrapped functions. I also tried with decorators and a super class. In the
end I saw a simple way to do it with the mixin, so went for that. I'd be
happier with a decorator solution for the reasons you mention.

> Also, it's not really clear to me how cooperative invocations of wrap() are
> meant to work with inheritance.  Using a decorator on methods which were
> intended to be deferred wouldn't fully solve that problem (you've still got
> to sort out what order methods get restored in, or if there are multiple
> calls to wrap() in different places in the inheritance tree which methods
> go with which Deferreds) but it would at least provide a convenient
> starting place to put that information.

Yes....

> >            self.waiting = []
> >            self.stored = {}
> 
> I'd make these attributes private if I were you.  I am pretty sure that
> you don't ever want application code poking around in there :).

Right. A bad habit of mine. I did at least think of this afterwards :-)

> >            def restore(_):
> >                for method in self.stored:
> >                    setattr(self, method, self.stored[method])
> 
> The reference you're cleaning up here has some edge-cases.  For example,
> if some other code comes along and grabs what it thinks is a regular
> bound method from your instance, and then invokes it after the Deferred
> has completed, it will still have the original method.

You mean it will still have the wrapped method, I think. Agreed that's a
problem. I was originally going to look at d.called in the mixin class to
short-circuit the wrapped behavior if the deferred had fired. I should do
something like that, else the deferred from the wrapper will never fire -
which is what I think you're saying.

> There are also some less severe, but potentially very confusing issues
> with making every instance of your class always participate in a
> bazillion circular references.

I don't think I fully understand this. The instance of my class only has
wrapped functions for a (typically?) very short time. I don't see the
circular references, but OTOH I haven't thought about that at all...

> By itself, this isn't really worth worrying about (Python added a garbage
> collector for a reason, after all) but it has historically been
> problematic in areas like making debugging memory leaks tricky.
> Especially when the circular references run through stack frames which
> refer to Deferreds :).  So if you do dynamically replace a method on a
> class, it's better to clean it up with delattr() than a subsequent
> setattr().

You mean "then" a subsequent setattr, right?

And thanks, I didn't know that at all.

> This wrapper doesn't preserve function metadata

Right - that was something I deliberately left out. I've even used
t.p.u.mergeFunctionMetadata in the past :-)

> I think some other decorator libraries have cuter / easier to use
> implementations of the same thing, this problem is not unique to
> Twisted).

There's also functools.update_wrapper

> >            d.addCallback(restore)
> 
> Here, on the final line, we come to the more serious problem of this
> approach: there's no error handling.  If the underlying Deferred
> encounters an errback, then all methods of this class will forever return
> Deferreds that never fire.

Ah yes :-)

Side note: I recently escaped from a fundamentalist religious organization
amongst whose axioms are "There Are No Accidents", and "Everything Happens
For A Reason". Accordingly, they code in a version of Python that doesn't
even have exceptions, and use a fork of Twisted in which deferreds don't
have an errback chain.

Ahem. You're right. At the very least I should add an errback that errbacks
the waiting calls, probably restores the methods, and returns the failure.
That's not a full solution, but it's better.

> Of course you could chalk up a failed connect Deferred to a failed
> startup and just reboot the process, but that pollutes your callers with
> knowledge of whether they're calling methods during startup.

Yes. There's also the question of what failure to pass to them, suppose I
do errback them (which I think I should).

> More importantly and realistically though - there's something that
> happens *later* which is never covered.  What happens when we *lose* the
> connection to the database?  Assuming a sensible underlying interface,
> everybody starts getting errbacked Deferreds, but in most systems like
> this you want some recovery facility.  And then you're not talking about
> just interesting behavior of __init__, but potentially of every method on
> the entire class.

Yes, and now we're into more interesting territory, where your state
machine solution would be nice to have.

> As I mentioned above, we've implemented this mechanism in other projects.
> One of them is Axiom.  Axiom has a batch-processing service which is a
> process pool that starts on demand, and tries to present a consistent
> interface to its callers regardless of what state the actual processes are
> in.  (This was written in no small part because we were using libraries
> which were flaky and unreliable and wanted to isolate their usage behind a
> nice facade which wouldn't freak out if they segfaulted.)
> 
> You can see a usage of our library here, which I believe meshes with your
> use-case:
> 
> http://divmod.org/trac/browser/trunk/Axiom/axiom/batch.py?rev=15165#L709

OK, I'll go check this out. I'd have done it already but commenting on it
here would make this reply even longer.

> I was originally going to include it inline here, but it turned out to be
> >100 lines of code to get the whole idea across, so I put it up here:
> 
>     http://divmod.org/trac/browser/sandbox/glyph/modality.py?rev=17275
> 
> This is still missing a lot of details, like for example handling truly
> failed connections (i.e. invalid credentials), timeouts and backoff,
> redirects, etc.  Still, I hope it's somewhat obvious how you would add
> additional methods beyond "bork()" to that example.

Yes. It looks clean and nice. But I'll have to spend time reading it again
and thinking about it to say more.

> It would be possible, I think, to implement a layer on top of epsilon.modal
> which would provide this pattern exactly so that you just need to plug in
> your retransmission and connection rules rather than doing it for every
> different application and protocol; that would be really cool.

Yeah, I'd use it :-)

> epsilon.modal is missing a few useful features, and has a few bugs.  I'm
> hoping that by drawing attention to it we can get some contributions from
> people who are enthusiastic about abstractions like this (hi, Terry! ;-))

Glyph, hi!

> >I quite like this approach. (...) It's nice because you don't reply with
> >an error and there's no need for locking or other form of coordination -
> >the work you need done is already in progress, so you get back a fresh
> >deferred and everything goes swimmingly.
> 
> IMHO this is a very important property.  The high-level abstract API should
> really have fewer failure modes and differing states for its callers to
> know about than the lower-level one - really that's the whole point :-).

Yes, agreed.

> >Comments welcome / wanted.
> 
> Enough comments for you? ;-)

Yep, and thanks for taking so much time and going into detail. I'll no
doubt continue to think about this. And I'll go look at epsilon.modal.

Terry