spine-tools / Spine-Database-API

Database interface to Spine generic data model
https://www.tools-for-energy-system-modelling.org/
GNU Lesser General Public License v3.0
6 stars 5 forks source link

Don't start Spine DB server automatically #187

Closed soininen closed 1 year ago

soininen commented 1 year ago

The database server gets started automatically when the spine_db_server module is loaded. This is problematic in many ways:

I suggest we redesign the database server such that it is only started by Spine Engine or other clients as needed. Perhaps a stand-alone Python application that could be run in a dedicated process?

manuelma commented 1 year ago

We don't start the database server automatically - those are started whenever an item is executed that needs contact with a DB. There is one server per (parallel) item execution and the server is closed when the execution is finished.

What we start automatically are the so-called 'ordering manager' and 'server manager' which are used to control write-order and to manage abovementioned servers. I think it's a good idea to start and join those processes with the spine engine - should be easy to do.

soininen commented 1 year ago

What we start automatically are the so-called 'ordering manager' and 'server manager'

Looks like I've still lot to learn about the db server. Thanks for the clarification!

ptsavol commented 1 year ago

I was just about to make this issue but @soininen beat me to it. This really is a problem. I can't do any tests at the moment. Looks like this line from spinedb_api.spine_db_server import closing_spine_db_server, quick_db_checkout in spine_engine.project_item.project_item_resource.py starts two additional Python processes (ordering manager and server manager, i guess). This means that any test that imports anything from spine_engine starts those two extra processes. This leads to very hard to debug errors and a waste of resources. I think we should not open processes at import time just in case but only when needed.

Related issue #185

manuelma commented 1 year ago

Aren't tests passing at the moment?

soininen commented 1 year ago

Aren't tests passing at the moment?

If I run all of them, then yes. But if I run a single TestCase or just a single test, it will hang forever. It makes test development painful. I also have issues with scripts that utilize Toolbox plotting methods.

ptsavol commented 1 year ago

Issue #185 still remains. It's kind of slow to run all tests instead of just the ones you need.

manuelma commented 1 year ago

I see, sorry about that. I haven't experienced any of those problems but I guess I'm on linux where multiprocessing's implementation is arguably better. Anyways the solution of starting and joining the two processes together with the engine should solve it?

soininen commented 1 year ago

Anyways the solution of starting and joining the two processes together with the engine should solve it?

Yes, I'd expect starting and stopping the processes on-demand would be the solution.

manuelma commented 1 year ago

I can do it quickly - or are you already working on it?

soininen commented 1 year ago

I hope you don't mind the assignment :)

manuelma commented 1 year ago

Done. It's gonna be a bit awkward because it required two PRs, one in spinedb_api and one in spine_engine, but the latter needed the former to be merged into master for tests to pass. So I don't know, maybe we'll see a failed action.

But I run all repos' tests in both linux and windows before merging, so we should be ok.

Hope it helps your development. Please close if solved or let me know if I missed something.