Roundup Tracker - Issues

Issue 1703116

classification
Title: session cleanup causes exceptions on postgresql
Type: Severity: normal
Components: Database Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: jpend Nosy List: forsberg, jpend, schlatterbeck
Priority: high Keywords:

Created on 2007-04-18 17:12 by forsberg, last changed 2007-09-25 02:46 by jpend.

Files
File name Uploaded Description Edit Remove
serialization-psycopg.diff forsberg, 2007-09-03 18:51 (Ugly) patch that seems to fix problem. Now with rollback.
back_postgres.diff jpend, 2007-09-21 21:04 a variant on Erik's earlier patch
Messages
msg2424 Author: [hidden] (forsberg) Date: 2007-04-18 17:12
In http://thread.gmane.org/gmane.comp.bug-tracking.roundup.user/7135, a problem with exceptions from the session cleaning is discussed. A fix was commited in revision 1.34 of back_postgresql.py, but unfortunately the fix doesn't work as the exception is raised in cursor.execute(), not during commit (see link to original bug report for full traceback)
msg2425 Author: [hidden] (schlatterbeck) Date: 2007-08-15 09:50
I'm seeing the same on a busy tracker several times a day.
System: Debian Etch (Stable) with psycopg 1.1.18

If I try to ignore the errors (by creating a sessions_postgresql.py where I catch the exception) I'm getting from time to time -- (but the original errors on session management seem to be gone) errors about otk (one time key) management:

Traceback (most recent call last):

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 258, in inner_main
    self.determine_user()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 448, in determine_user
    self.clean_sessions()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 353, in clean_sessions
    self.db.getOTKManager().clean(now)

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/backends/sessions_postgresql.py", line 45, in clean
    BaseBasicDatabase.clean(self, now)

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/backends/sessions_rdbms.py", line 92, in clean
    self.name, self.db.arg), (old, ))

ProgrammingError: ERROR:  current transaction is aborted, commands ignored until end of transaction block

delete from otks where otk_time < 1186448402.04

So I guess, the OTK and Session management code should run in its own transaction. Is somebody fluent enough in the psycopg framework to create a separate cursor with its own transaction for the session management? It looks to me like the cursor in question is running in auto-commit mode, otherwise the error would be raised on commit not on cursor.execute?

For the record, these are the tracebacks seen with the original code,
three different tracebacks with the original code:

Traceback (most recent call last):

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 258, in inner_main
    self.determine_user()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 448, in determine_user
    self.clean_sessions()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 352, in clean_sessions
    sessions.clean(now)

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/backends/sessions_rdbms.py", line 92, in clean
    self.name, self.db.arg), (old, ))

ProgrammingError: ERROR:  could not serialize access due to concurrent update

delete from sessions where session_time < 1184374803.64


Traceback (most recent call last):

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 258, in inner_main
    self.determine_user()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 486, in determine_user
    sessions.updateTimestamp(self.session)

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/backends/sessions_rdbms.py", line 85, in updateTimestamp
    (now, infoid, now-60))

ProgrammingError: ERROR:  could not serialize access due to concurrent update

update sessions set session_time=1184837598.64 where session_key='MTE4MzYyMDMxNy40NTAuMjI0MjUyNDMzNzMy'
            and session_time < 1184837538.64


Traceback (most recent call last):

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 258, in inner_main
    self.determine_user()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 448, in determine_user
    self.clean_sessions()

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/cgi/client.py", line 354, in clean_sessions
    sessions.set('last_clean', last_use=time.time())

  File "/roundup/sw/python2.4//lib/python2.4/site-packages/roundup/backends/sessions_rdbms.py", line 73, in set
    c.execute(sql, args)

ProgrammingError: ERROR:  could not serialize access due to concurrent update

update sessions set session_value='{''last_use'': 1184806806.3256691}' where session_key='last_clean'

msg2426 Author: [hidden] (forsberg) Date: 2007-08-30 19:24
See attached file for a rather ugly fix (it does postgres-specific stuff outside the postgres backend) that we've applied on the roundup running http://bugs.python.org/. 

I don't think this patch is good enough for inclusion in roundup HEAD.
File Added: serialization-psycopg.diff
msg2427 Author: [hidden] (forsberg) Date: 2007-09-03 18:51
File Added: serialization-psycopg.diff
msg2428 Author: [hidden] (jpend) Date: 2007-09-21 21:04
This seems like a pretty common problem for people using the postgres backend. If we're going to release 1.4 sometime soon I'd rather see this included as-is than wait for a better version that may not materialize :)

That said, I've attached (a completely untested; I'll try that tonight :) version of Erik's patch that pushes this more into the Postgres backend. The PG backend provides its own getSessionManager() method that returns a custom Session object that handles the try/except during a set. Erik, do you think this approach is a better candidate for inclusion in HEAD?

If so (and provided my testing goes well), I'll commit it (or something like it) in the near future.
File Added: back_postgres.diff
msg2429 Author: [hidden] (forsberg) Date: 2007-09-24 18:30
I think your patch looks fine, at least at visual inspection :-). It's definitely a better solution than my patch, so if you ask me, you should go ahead and commit it! 

(You might want to get rid of the last chunk, which is only a whitespace difference).
msg2430 Author: [hidden] (jpend) Date: 2007-09-25 02:46
I fixed some blatant syntax errors and have committed this to CVS.
History
Date User Action Args
2007-04-18 17:12:48forsbergcreate