Opened 5 years ago

Closed 3 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)

ora_version.diff (6.1 KB) - added by Anssi Kääriäinen 5 years ago.
ora_version.2.diff (6.2 KB) - added by Anssi Kääriäinen 5 years ago.

Download all attachments as: .zip

Change History (7)

Changed 5 years ago by Anssi Kääriäinen

Attachment: ora_version.diff added

comment:1 Changed 5 years ago by Luke Plant

Needs documentation: unset
Needs tests: unset
Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Accepted. The 'oracle_version' property could be simplified through use of our 'cached_property' utility I think.

comment:2 Changed 5 years ago by Anssi Kääriäinen

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 Changed 5 years ago by Anssi Kääriäinen

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 Changed 5 years ago by anonymous

Patch needs improvement: unset

Changed 5 years ago by Anssi Kääriäinen

Attachment: ora_version.2.diff added

comment:5 Changed 3 years ago by Aymeric Augustin

Resolution: fixed
Status: newclosed

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.

Note: See TracTickets for help on using tickets.
Back to Top