Code

Opened 2 years ago

Closed 6 months ago

#17291 closed Cleanup/optimization (fixed)

Oracle: cache the server version when first needed

Reported by: akaariai 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 akaariai 2 years ago.
ora_version.2.diff (6.2 KB) - added by akaariai 2 years ago.

Download all attachments as: .zip

Change History (7)

Changed 2 years ago by akaariai

comment:1 Changed 2 years ago by lukeplant

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

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

comment:2 Changed 2 years ago by akaariai

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 2 years ago by akaariai

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

  • Patch needs improvement unset

Changed 2 years ago by akaariai

comment:5 Changed 6 months ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.