#19274 closed Cleanup/optimization (fixed)
Separate DB connection creation and session state initialization
Reported by: | Anssi Kääriäinen | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently when creating a new connection we create the connection and initialize it in one go (in _cursor()). Even if a backend splits getting a new connection and initializing the session variable this is not done consistently between backends.
The reason for this split is that this gives nice access point for external pooling implementations. A pool can wrap the backend and the methods "get_new_connection()" and "close()". Instead of actually creating new connections and closing the connection we can just get a connection from the pool and initialize the state of it. The real backend doesn't need to know anything about the pool wrapper.
The pool wrapper implementation would be something like this:
def get_new_connection(): if pooled_connections_available(): return connection from pool else: return super().get_new_connection() def init_connection(): do possible pool specific initialization super().init_connection() def close(): put the connection back to pool
I think this would allow connection pools for Django but with no need to implement them in core. In addition this should add readability of ._cursor() implementations.
Change History (5)
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 12 years ago
I have now a working implementation in: https://github.com/akaariai/django/compare/ticket_19274
The work divides into three parts:
- Backend conversion as mentioned in the ticket description
- Removing the assumption that connection's self.settings_dict changes will be reflected in the global settings
- Random test fixed (one Oracle specific fix, some fixes into how to create additional connections for testing and some fixes to accessing the connection.connection variable)
After the fixes the work in https://github.com/akaariai/django_pooled/ passes on all core backends. The pooling implementation in django_pooled is somewhere between ugly and hideous. What is interesting is that django_pooled shows that one can have generic pooling implementation outside core, and that it actually works.
The separation of self.settings_dict from global settings is needed for the above pooling implementation (and I believe any pooling implementation wants that). The basic problem is that you need to tell Django to use the pool in ENGINE, and then you need to have the real engine somewhere (OPTIONS -> WRAPS in django_pooled). You need to alter the settings before passing to the real backend, but you can't do that to the global settings, otherwise the information is lost for next connection creation.
Gis backends aren't converted yet.
comment:3 by , 12 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
I have update the patch series at https://github.com/akaariai/django/compare/ticket_19274
Gis isn't yet updated, though it seems the only change needed is in spatialite. In addition I would like to remove some checks done against the ENGINE string, this isn't nice for subclassers.
The full test suite does pass using https://github.com/akaariai/django_pooled/. The implementation in django_pooled isn't meant for any kind of production use, but it does prove that creating a connection pool outside core is possible.
I am planning to push the patch series as-is. The changes are mostly mechanical - moving code from ._cursor() to the new methods. Still, it would be great if somebody had the time to double-check my changes, I am pretty good at making stupid little mistakes in this sort of thing...
comment:4 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Sounds good to me.