Code

Opened 7 years ago

Closed 7 years ago

#3459 closed (fixed)

SET TIME ZONE called too often for postgresql_psycopg2 backend

Reported by: Jack Moffitt <metajack@…> Owned by: adrian
Component: Database layer (models, ORM) Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Analyzing our database logs I found that 25% of all traffic were calls to SET TIME ZONE. This is currently called for every cursor when using the postgrsql_psycopg2 backend. This only needs to be called when the connection is established. All cursors will get the correct timezone if it is set in the connection.

I've attached a patch which calls SET TIME ZONE only on new connections. This eliminates a lot of wasted database traffic and can result in a large performance gain for loaded systems using this backend.

Attachments (3)

settimezone.diff (1.1 KB) - added by Jack Moffitt <metajack@…> 7 years ago.
patch to call SET TIME ZONE only on new connections instead of for every cursor
combined.diff (1.5 KB) - added by Jack Moffitt <metajack@…> 7 years ago.
updated patch combined with fix for #3460
tztest.py (657 bytes) - added by Jack Moffitt <metajack@…> 7 years ago.
test script

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by Jack Moffitt <metajack@…>

patch to call SET TIME ZONE only on new connections instead of for every cursor

comment:1 Changed 7 years ago by Jack Moffitt <metajack@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Note: According to postgresql 8.1 docs, this is correct: http://www.postgresql.org/docs/8.1/interactive/sql-set.html

The SET will take effect for the entire session unless something else calls set. This is regardless of whether it is inside a transaction or not. However, the patch needs to be changed to do a COMMIT otherwise the change will not take effect if this cursor's first transaction ends in a rollback. I've attached a modified patch which also combines the fix for #3460, which needs a similar setup.

I've also attached a test script to show that this works regardless of the isolation level used.

Changed 7 years ago by Jack Moffitt <metajack@…>

updated patch combined with fix for #3460

Changed 7 years ago by Jack Moffitt <metajack@…>

test script

comment:2 Changed 7 years ago by SmileyChris

  • Triage Stage changed from Unreviewed to Ready for checkin

According to docs this looks right and the code looks good.

There may be some gotchas that haven't been considered, but I'll let a committer mull over those and change the state back if required. :P

comment:3 Changed 7 years ago by Marc Fargas <telenieko@…>

For the record:
Discussion about that was taken in: there

comment:4 Changed 7 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

(In [4573]) Fixed #3459: Django no longer runs SET TIME ZONE for every query when using Postgres. This should result in a pretty noticible speedup for Postgres users, so many thanks to Jack Moffitt for the patch.

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.