Opened 12 years ago
Closed 11 years ago
#17291 closed Cleanup/optimization (fixed)
Oracle: cache the server version when first needed
| Reported by: | Anssi Kääriäinen | Owned by: | nobody |
|---|---|---|---|
| Component: | Database layer (models, ORM) | Version: | |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
As discussed in this django-developers thread, it would be good from performance standpoint to cache the version information when first needed, and then use that information for every connection taken from that DB. The caching is per database alias.
There is also some cleanups included in the patch. The main thing is that I added supports_regex DatabaseFeature, which is false for Oracle v9.
The attached patch does pass lookup tests. It is about 20% faster on taking 1000 connections and immediately closing them.
I do get one test failure in basic tests, but I do get the same failure in test_unicode_data on master, too. Probably something wrong with my setup. I am running the full test suite currently, but I can't report about results until Monday when I am next at work.
Unfortunately I can't test this on Oracle v9.
Attachments (2)
Change History (7)
by , 12 years ago
| Attachment: | ora_version.diff added |
|---|
comment:1 by , 12 years ago
| Patch needs improvement: | set |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 12 years ago
Yes, it seems that cached_property would simplify the implementation. I will post an updated patch on Monday. Results from the test suite should be available then, too.
I wonder if making the dbwrapper.ops a cached_property would be a wise thing to do. Seems like that would simplify the oracle backend some more, and would get rid of additional lines in _cursor() method. But I will not piggy-bag that into this ticket.
comment:3 by , 12 years ago
Ok, updated patch attached, now with cached property. I got one error on the full test suite with this patch:
- Unicode error on basic tests (happens also without the patch)
As far as I am concerned, this ticket is ready for committer. I haven't tested this on Oracle 9 though, but I think this will work just fine on that version.
comment:4 by , 12 years ago
| Patch needs improvement: | unset |
|---|
by , 12 years ago
| Attachment: | ora_version.2.diff added |
|---|
comment:5 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
This issue is fixed: DatabaseWrapper.oracle_version is now implemented as a cached property.
Anssi, if there are other changes in the patch that are still relevant, feel free to reopen this ticket or open another one.
Accepted. The 'oracle_version' property could be simplified through use of our 'cached_property' utility I think.