-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add connection pooling #483
Comments
we're also going to need some kind of timer delay for waiting for a connection to be open |
Hello @josephmancuso! Another enhancement I can suggest is to make Masonite thread-safe. If I open a transaction in a thread and another thread is doing queries, I get orm/src/masoniteorm/connections/ConnectionResolver.py Lines 4 to 8 in 8d907a7
Is there any way to segregate the open connections in order that each thread can access only its own connections? |
If you can give some code examples I can test it out and figure out the issue. The connection resolver just holds the available connection classes. I don't think that should be a thread issue but I could be wrong |
I will try to provide a minimal example as soon as possible since I have a quite complex application. orm/src/masoniteorm/connections/ConnectionResolver.py Lines 47 to 62 in 8d907a7
Then, the connection is reused when a connection is required (line 68-69): orm/src/masoniteorm/connections/MySQLConnection.py Lines 45 to 83 in 8d907a7
In summary, I suppose that connections are shared between different threads... |
Hmm if you are using transactions than that's possible it's trying to access a connection in another thread |
Would this effect be due to every call to the db when creating a model or querybuilder instance or just getting DB (connectionresolver with config) for Transaction use generates a new instance of the ConnectionResolver? Shouldn't the ConnectionResolver be cached in some way to utilise the same connection during multiple consecutive queries? Or am I just looking at this wrong? |
So theres a few definitions that need to be defined. Theres a few different "connection" references here.
With queries we open and close a connection on every single query. I would assume this would be thread safe since the thread doing the query also should be the one creating and closing the connection class (3.). With transactions the current implementation is to store the "connections" (the 3. here) and then save the connection globally so we can keep the transaction open for the duration of the transaction and then once the transaction is committed or rolled back we close this open transaction from the connection. I think what @federicoparroni is suggesting is that doing that can result in 2 threads sharing the same "connection" class (3.) and then one thread closing the connection while another thread still is using it |
Yes @josephmancuso, it's exactly that situation. My current workaround for this issue is to create ad-hoc connections with unique names for each thread, so that when a transaction begins, it is only used by that single thread. I'm using the |
I think the solution is to probably use SAVEPOINTS instead of using the cursor on the connection class but i'll have to research that a bit. |
sqlalchemy uses something called a "session factory" which creates a new sqla session if one does not exist, or returns the current session if it does. Session instantiation involves obtaining a new connection. This session object provides thread-safe property access to things like the current transaction and its db connection. Something akin to that could be of use here – but I think thread safety should probably be another issue altogether though it's important to think about in this context. |
Right now, Masonite ORM makes a connection and drops a connection on every query call. This is fine in most instances but if you have several application making the same connections to the same database, you'll want to set a connection_pool_limit. Like 100 connections. Then we can pull from the connection pool rather than making a new connection every time
The text was updated successfully, but these errors were encountered: