chime-experiment / dias

A data integrity framework
https://dias.readthedocs.io/
GNU General Public License v3.0
2 stars 0 forks source link

There is a ConnectionError when connections are shared between threads. #158

Open anjakefala opened 4 years ago

anjakefala commented 4 years ago

Current workaround: restart DIAS.

Don's proposed fix:

Tasks shouldn’t be cacheing DB connectors by opening them in setup().  

A new connector needs to be opened each time run() is executed.
ketiltrout commented 4 years ago

I don't think its the tasks per se that are really at fault here. The problem is if one of them creates a finder object via self.Finder (being derived from CHIMEAnalyzer) then that Finder will automatically create a new connector when it first runs. When the analyzer exits, the connector just sits around and then next time the finder gets re-used, it thinks the connector is still good an just re-uses it despite maybe being in a different thread now.

So I think what needs to happen is CHIMEAnalyzer needs to add a hook somehow that runs after its the subclasses' run() methods get executed by the scheduler. The hook should call chimedb.close() to ensure any connector that was opened (perhaps inadvertantly) by run() are closed.

Or we need better task context management. (i.e. this would be fixed by changing from thread to process pool, which is definitely not the easy way to fix this.)

ketiltrout commented 4 years ago

There's also a few sqlite connectors being opened in setup() methods. I don't know if those are thread safe.

ketiltrout commented 4 years ago

My comment about analyzers doing manual opens on CHIME DB in setup() I don't think is true anymore.

Also I kind of thought the thread-safety stuff we implemented in chimedb would have dealt with this. So maybe these errors are legitimate network transport errors. Then the real solution is to catch the connection error and try to recover from it by attempting to closing and re-open.