Comment 12 for bug 531834

Revision history for this message
Stuart Bishop (stub) wrote :

A bad assumption made me think that was what is happening.

I can't reproduce the hanging connections locally with launchpad trunk. Looking back at the mailing list thread it looks like it would be difficult to reproduce unless under load with a proper sized database:

""" This works by having the DB URI as a config variable that returns the
    appropriate value according to the presence of the read-only.txt file,
    together with a publication hook that forces the storm stores to
    reconnect whenever there's a mode switch.

    However, storm (through ZStorm) shares storm.database.Database instances
    across threads (without saying so in the docs) and our custom version of
    that class (LaunchpadDatabase) may change its DB URI (when there's a
    config change) after instantiation[2]. This, in itself, is a problem,
    but since we don't do runtime config changes except when running the
    test suite (which is single-threaded), it's never affected us.

    My changes for the read-only switch, though, introduced a way of
    changing config values at runtime, thus exposing the problem.

    This means there's a race condition when switching to read-only mode,
    when a thread kicks in after another thread has noticed the config
    change and reconnected its stores, thus causing the shared instance of
    LaunchpadDatabase to change its DB URI. When other threads kick in,
    they'll think they're connected to the correct DB (remember we use the
    DB URI stored in LaunchpadDatabase for that) and won't tell their stores
    to reconnect, leaving open connections to DBs that should have no
    connections.
"""

The good news is that my fix sorts this too I think, as we no longer use the DB URI to detect if we have switched too or from read-only mode (and as an additional benefit, we no longer need to poke at Storm internals per the existing comment in the code).