[storm] probable bug in storm/0.19/storm.twisted.transact causes high db locks.

Free Ekanayaka free.ekanayaka at gmail.com
Mon Jan 21 14:28:08 UTC 2013


Hi Giovanni,

thanks for reporting this.

On Sat, Jan 19, 2013 at 2:01 PM, Giovanni `evilaliv3` Pellerano
<evilaliv3 at globaleaks.org> wrote:
> While debugging some code of the Globaleaks software i've probably
> found a bug related to storm integration with twisted.
>
> in reference to storm/0.19/storm.twisted.transact, the
> time.sleep(seconds) inside @transact decorator causes a long db lock
> in case of exceptions.

I'm not sure to understand what you mean exactly. The code itself
seems fine (but see below), as it aborts the transaction before
sleeping, so it won't cause a "db lock" (unless there's something
weird going on with your user code or with sqlite).

On the other hand I do see how this behavior is not
performance-friendly for scenarios with a lot of concurrent requests
AND database contention at the same time, because that sleep() will
cause the thread to hang and other requests will have to wait.

> in my opinion, the retry mechanism needs to be implemented outside of
> the @transact method and not vice versa.        furthermore a sleep
> expressed in seconds is probably too long for a transaction retry.

This feature is indeed weird, it was put there for compatibility with
some Zope code. It should be probably taken out of @transact and put
in a different decorator, like you suggest, which ideally releases the
thread and uses reactor.callLater instead of a blocking sleep. I think
I'll try that.

I also agree that sleeps should be more in the order of milliseconds
than seconds.

> here are some reference to the issue and to the temporary fix:
> https://github.com/globaleaks/GLBackend/issues/44
> https://github.com/globaleaks/GLBackend/commit/aec7e6902ed64ba33f856ca6bd69aa049a6cf64d

The workaround that you put in place seems good, and it's actually not
even a workaround, as the number of retries is configurable, so
setting it to zero is perfectly acceptable. Probably zero should have
been the default, to avoid surprises like this. My bad, I'm sorry that
it caused you troubles.

Cheers,

Free



More information about the storm mailing list