Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
anjakefala opened this issue Aug 31, 2020 · 3 comments
Open
Labels
analyzer Add a new Analyzer bug Something isn't working scheduler Related to the dias daemon

Comments

@anjakefala
Copy link
Contributor

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.
@anjakefala anjakefala added the bug Something isn't working label Aug 31, 2020
@ketiltrout
Copy link
Member

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
Copy link
Member

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

@ketiltrout
Copy link
Member

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.

@ketiltrout ketiltrout added analyzer Add a new Analyzer scheduler Related to the dias daemon labels Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer Add a new Analyzer bug Something isn't working scheduler Related to the dias daemon
Projects
None yet
Development

No branches or pull requests

2 participants