Opened 16 years ago

Last modified 3 months ago

#7732 assigned New feature

Oracle Backend with SessionPool

Reported by: halturin Owned by: Suraj Shaw
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: yandex-sprint oracle session pool
Cc: halturin@…, nleschev@…, Erin Kelly, Matt Boersma, dbergstr@…, erny@…, Dan Davis, Christopher Jones, Suraj Shaw Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This new backend uses Oracle session pool to improve database performance in threaded environment. Overall speedup for large amounts of small queries is 2-3 times.

Attachments (8)

oracle_pool.tar.gz (10.4 KB ) - added by halturin 16 years ago.
Tarball with oracle_pool db backend
oracle_pool.diff (4.3 KB ) - added by jbronn 16 years ago.
Oracle pooling patch derived from tarball.
oracle_pool.2.diff (4.1 KB ) - added by Erin Kelly 16 years ago.
Modified patch with print statements removed
oracle_pool.3.diff (4.7 KB ) - added by nleschev 16 years ago.
oracle_pool.4.diff (7.3 KB ) - added by dbergstr@… 16 years ago.
Patch against revision 10578 (1.1beta). Use settings.DATABASE_OPTIONS, general code cleanup.
oracle_pool.5.diff (7.4 KB ) - added by Matt Boersma 16 years ago.
Modified patch against trunk [10832] that passes the Django test suite
oracle_pool.6.diff (3.9 KB ) - added by erny 15 years ago.
Oracle Pool for 1.0.X branch
Session Pool Backend in Django For Oracle (1).docx (22.6 KB ) - added by Suraj Shaw 10 months ago.
Design specification

Download all attachments as: .zip

Change History (60)

by halturin, 16 years ago

Attachment: oracle_pool.tar.gz added

Tarball with oracle_pool db backend

comment:1 by Sung-jin Hong, 16 years ago

milestone: post-1.0
Needs documentation: set
Patch needs improvement: set
Triage Stage: UnreviewedDesign decision needed

Can you modify the files to conform with Django Patch Style?

And I think adding a new backend needs design decision. By patching the existing oracle backend and making pooling an option seems to be better. Making milestone post-1.0 since modifying or adding feature to the current backend is currently out of scope of 1.0 release.

by jbronn, 16 years ago

Attachment: oracle_pool.diff added

Oracle pooling patch derived from tarball.

by Erin Kelly, 16 years ago

Attachment: oracle_pool.2.diff added

Modified patch with print statements removed

comment:2 by Erin Kelly, 16 years ago

I'm trying to run this through the test suite, and I'm seeing lots of non-deterministic test failures. When I try to run the full test suite, I get an early error where it's setting up the test database and apparently trying to reset a sequence that doesn't exist. When I try to run just the basic test, the test runs, but lots of queries return dramatically incorrect results. It's not obvious where this is coming from, but it needs to be addressed before this can be committed.

comment:3 by nleschev, 16 years ago

Cc: halturin@… nleschev@… added

We have included all changes in this patch(oracle_pool.3.diff). Also has been added 4 settings.
DATABASE_SESSION_POOL=[True/False]
DATABASE_SESSION_POOL_MIN=1
DATABASE_SESSION_POOL_MAX=3
DATABASE_SESSION_POOL_INCREMENT=1

by nleschev, 16 years ago

Attachment: oracle_pool.3.diff added

comment:4 by Erin Kelly, 16 years ago

Cc: Erin Kelly added

comment:5 by Matt Boersma, 16 years ago

Cc: Matt Boersma added

comment:6 by Matt Boersma, 16 years ago

Keywords: oracle session pool added

comment:7 by (none), 16 years ago

milestone: post-1.0

Milestone post-1.0 deleted

by dbergstr@…, 16 years ago

Attachment: oracle_pool.4.diff added

Patch against revision 10578 (1.1beta). Use settings.DATABASE_OPTIONS, general code cleanup.

comment:8 by anonymous, 16 years ago

Cc: dbergstr@… added

comment:9 by Matt Boersma, 16 years ago

milestone: 1.2
Needs tests: set
Owner: changed from nobody to Matt Boersma
Status: newassigned
Triage Stage: Design decision neededAccepted

I'm still not able to get cx_Oracle's SessionPool to behave correctly for the entire Django test suite. I've tried the 4/16/09 patch here as well as my own implementations, but we quickly get test case failures that I've had a very hard time tracking down.

I have not given up--I think this should offer obvious speed improvements. I'm marking this as

comment:10 by Matt Boersma, 16 years ago

...(continued) I'm marking this as needed for the next big release.

comment:11 by dbergstr@…, 16 years ago

I've never looked at the test suite, nor do I have a clue how cx_oracle manages connections and transactions, but perhaps problems arise because some of the tests don't do a commit? The connection would then go back into the pool with an open transaction, and when it was next pulled out of the pool, the state in that connection would be different from what is expected. In general, using any pooled resource can expose sloppy practices in the calling code (failure to properly initialize shared memory, db connection leaks, etc.).

by Matt Boersma, 16 years ago

Attachment: oracle_pool.5.diff added

Modified patch against trunk [10832] that passes the Django test suite

comment:12 by erny, 16 years ago

Cc: erny@… added

comment:13 by erny, 16 years ago

Can we have a bit of feedback about if the patch is supposed to work correctly now and it's effectivity?

comment:14 by Erin Kelly, 16 years ago

The patch works as far as we've seen. The issues that were arising in the test suite had to do with the test setup attempting to disconnect, change the connection parameters, and reconnect, which failed because of the pool. For the same reason, I suspect it will take some work to make this compatible with the multidb branch, but otherwise it's usable.

comment:15 by erny, 15 years ago

The patch didn't work for me for 1.0.X branch rev10865. I attach al alternative patch for this version.

by erny, 15 years ago

Attachment: oracle_pool.6.diff added

Oracle Pool for 1.0.X branch

comment:16 by erny, 15 years ago

I found that the used SessionPools getmode parameter may no match everybody's requirements. This should be a parameter. Would be great if this really was a parameter of p.acquire()

comment:17 by erny, 15 years ago

Bad news: I get DatabaseError: ORA-03135: connection lost contact with cx_Oracle 4.4.1 which does not drop/release/reconnect the connections. After the all connections are acquired, I get: ORA-24418: Cannot open further sessions. As it's author in the cx_Oracle mailing list suggest, a pool.drop(connection) is not enough, as it raises another exception. The complete pool may be replaced.

comment:18 by edcrewe, 15 years ago

I have packaged up a version of this pooling backend, and made it available on pypi

see http://pypi.python.org/pypi/django-oraclepool

This includes some tests, and other extra little bits.
It has also been tested against 1.1 and 1.2

  • Ed Crewe

comment:19 by Alex Gaynor, 15 years ago

milestone: 1.21.3

We are past the feature phase of 1.2.

comment:20 by Luke Plant, 14 years ago

Severity: Normal
Type: New feature

comment:21 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:11 by Aymeric Augustin, 13 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:12 by Aymeric Augustin, 13 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:13 by Skylar Saveland, 12 years ago

I have what may be a more simple approach to the problem. I branched off of the current django master on github. I can easily remove the extraneous flake8 changes and squash into one commit for a pull request:

https://github.com/skyl/django/compare/7732-oracle-sessionpool

It appears that the tests will pass against Oracle (takes forever). However, I'm not that familiar with the ins and outs of how database backends work. If someone could look at my patch and let me know if there is something obvious I've missed, I would appreciate it. On a simple test webapp, I'm experiencing a significant speedup with the patch.

comment:14 by Anssi Kääriäinen, 12 years ago

It seems that allowing the use of the cx_Oracle inbuilt pooler is a good idea.

As for the implementation: is the use of class level pool variable really safe? It seems the pool instance is connection settings dependent, so having just one pool per project seems suspicious.

Also, the close() implementation should probably do a rollback before releasing the connection back to the pool.

comment:15 by Skylar Saveland, 12 years ago

I changed to 1 pool per alias; 1 pool per project was definitely wrong. I also changed to doing a self.connection.rollback() in close before releasing the connection.

comment:16 by erny, 12 years ago

I had the following problems in the past: The Oracle pools is a "per process" pool. Each Python Process gets its own pool. Supposing X processes (apache prefork, wsgi, etc.) with each Y min connections we get X * Y connections. So keep your min connections count low. With Oracle 11, there seems to be a server side session pool which should be much more convenient, but I didn't test it.

comment:17 by Skylar Saveland, 12 years ago

Unfortunately, I think there needs to be some error-checking on the connection after it is acquired from the pool. After some hours (I didn't configure the instance I'm running against so I'm not sure how long), I get ORA-02399: exceeded maximum connect time, you are being logged off; then, the pool continues to try to get the same connection, failing every time with, ORA-01012: not logged on. I think there are 2 options then.

1) put a CONN_MAX_AGE parameter in the SESSION_POOL settings dict and drop the connection if it is close enough to that age
2) ping the connection after it is acquired and drop it if it is not usable

(1) Age is not an attribute of the connection that I can see; so, it would be ugly to do our own book-keeping on the age of the connections in the pools we are holding. This does not cover other possible error scenarios.
(2) This would be a hit to performance every time a connection is acquired.

I think (2) would be easy enough to add. Other suggestions?

comment:18 by Omer Katz, 10 years ago

Since I believe that adding pooled backends was rejected from Django shouldn't this issue be closed?

comment:19 by Tim Graham, 8 years ago

Owner: Matt Boersma removed
Status: assignednew

comment:20 by Dan Davis, 6 years ago

Cc: Dan Davis added

I'm interested in this. I'm not quite on the point of assigning it, but one of my developers asked me see to return to what is needed to use Oracle DRCP. I tested it using django-oraclepool many years ago to determine whether there was any performance difference, from the client-side, between CONN_MAX_AGE and this feature. django-oraclepool is so old, that it seems I'd either need to fork it or return to this issue.

comment:21 by Mariusz Felisiak, 15 months ago

Cc: Christopher Jones Suraj Shaw added

comment:22 by Suraj Shaw, 10 months ago

Owner: set to Suraj Shaw
Status: newassigned

by Suraj Shaw, 10 months ago

Design specification

comment:23 by Suraj Shaw, 10 months ago

Last edited 9 months ago by Suraj Shaw (previous) (diff)

comment:24 by Suraj Shaw, 9 months ago

Has patch: unset
Needs documentation: unset
Needs tests: unset

comment:25 by Suraj Shaw, 9 months ago

Has patch: set
Patch needs improvement: unset

comment:26 by Mariusz Felisiak, 9 months ago

Patch needs improvement: set

comment:27 by Suraj Shaw, 9 months ago

Patch needs improvement: unset

comment:28 by Mariusz Felisiak, 9 months ago

Needs tests: set
Patch needs improvement: set

comment:29 by Suraj Shaw, 8 months ago

Needs tests: unset
Patch needs improvement: unset

comment:30 by Mariusz Felisiak, 8 months ago

Patch needs improvement: set

comment:31 by Suraj Shaw, 8 months ago

Patch needs improvement: unset

comment:32 by Sarah Boyce, 8 months ago

Patch needs improvement: set

comment:33 by Suraj Shaw, 8 months ago

Patch needs improvement: unset

comment:34 by Sarah Boyce, 8 months ago

Patch needs improvement: set

comment:35 by Suraj Shaw, 7 months ago

Patch needs improvement: unset

comment:36 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:37 by Suraj Shaw, 3 months ago

Patch needs improvement: unset

comment:38 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:39 by Suraj Shaw, 3 months ago

Patch needs improvement: unset

comment:40 by Sarah Boyce, 3 months ago

Patch needs improvement: set

comment:41 by Suraj Shaw, 3 months ago

Patch needs improvement: unset
Note: See TracTickets for help on using tickets.
Back to Top