Edgewall Software

Opened 10 years ago

Last modified 2 months ago

#9298 new defect

IntegrityError: duplicate key value violates unique constraint "session_attribute_pk"

Reported by: Christian Boos Owned by:
Priority: normal Milestone: next-stable-1.4.x
Component: general Version: 0.12b1
Severity: minor Keywords: session
Cc: Mitar Branch:
Release Notes:
API Changes:
Internal Changes:


Happened when loading a bunch of reports as anonymous.

Information système

Trac 0.12b1
Babel 0.9.5
Docutils 0.6
Genshi 0.6
psycopg2 2.0.8
Pygments 1.2.2dev-20100224
Python 2.5.2 (r252:60911, Oct 5 2008, 19:29:17)
[GCC 4.3.2]
pytz 2010g
setuptools 0.6c9
Subversion 1.5.1 (r32289)

Pile d'appel Python

Traceback (most recent call last):
  File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 512, in _dispatch_request
  File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 257, in dispatch
  File "build/bdist.linux-x86_64/egg/trac/web/session.py", line 88, in save
  File "build/bdist.linux-x86_64/egg/trac/db/util.py", line 60, in transaction_wrapper
  File "build/bdist.linux-x86_64/egg/trac/web/session.py", line 117, in delete_session_cookie
    """, attrs)
  File "build/bdist.linux-x86_64/egg/trac/db/util.py", line 142, in executemany
    return self.cursor.executemany(sql_escape_percent(sql), args)
IntegrityError: duplicate key value violates unique constraint "session_attribute_pk"

Attachments (0)

Change History (9)

comment:1 by pipern, 10 years ago

Similar to #7663? We are investigating having this same problem, so maybe it's not as closed as that ticket suggests (or a regression.)

comment:2 by pipern, 10 years ago

I think [9342] causes a regression on #7663 (simply by reading the code.)

The final fix would have to be taking into account keeping #9104 fixed, maybe by opening a new cursor.

comment:3 by Mitar, 9 years ago

Cc: Mitar added

comment:4 by Carsten Klein <carsten.klein@…>, 9 years ago

I think that the problem basically lies in the session update code itself:

125	            if not skip and self._old != self:
126	                if not items and not authenticated:
127	                    # No need to keep around empty unauthenticated sessions
128	                    db("DELETE FROM session WHERE sid=%s AND authenticated=0",
129	                       (self.sid,))
130	                db("""DELETE FROM session_attribute
131	                      WHERE sid=%s AND authenticated=%s
132	                      """, (self.sid, authenticated))
133	                self._old = dict(self.items())
134	                db.executemany("""
135	                    INSERT INTO session_attribute
136	                      (sid,authenticated,name,value)
137	                    VALUES (%s,%s,%s,%s)
138	                    """, [(self.sid, authenticated, k, v) for k, v in items])
139	                session_saved = True
  • Request 1 is processed, which will lock the db and delete all session attributes available and then it starts inserting the "new" attributes
  • Request 2 is processed, which in turn will also delete all session attributes currently available and it will start inserting the "new" attributes

So, while request 1 processing already removed the attributes and begun inserting the new ones, request 2 again removes the newly inserted session attributes and also begins inserting the same session attributes, eventually coming to a point where Thread/Process 1 just inserted the same attribute, which will then result in the experienced duplicate key exception.

Now, in pysqlite, the executemany() will be executed on a per parameter basis, so instead of inserting the bunch of parameters in one go, it will iterate over the existing parameters, see http://code.google.com/p/pysqlite/source/browse/src/cursor.c#640 where it reads

    while (1) {
        parameters = PyIter_Next(parameters_iter);

I presume that the database will not be locked for as long as the operation is processed but on a per insert statement basis.

One possible remedy I see here is to refactor the session handling code so that it will first create a difference set of the attributes in the session that have actually changed and then will delete and reinsert these attributes individually. The overhead is negligible due to the fact that the underlying pysqlite implementation also processes each insert statement on an individual basis.

comment:5 by Carsten Klein <carsten.klein@…>, 9 years ago

Of course, adding a try/except around the offending lines might do the trick, but it will not solve the inherent problem. See the regression mentioned in comment:2 induced by changeset:9342.

Here, you expect the exception and also loss of more current session data, if you reintroduce the try/except statements instead of only delete/insert the data that has actually changed.

comment:6 by Remy Blank, 9 years ago

The whole session update is wrapped in a transaction, so it should be atomic. But somehow, that's not the case.

comment:7 by Ryan J Ollos, 6 years ago

Milestone: next-minor-0.12.xnext-stable-1.0.x

comment:8 by Ryan J Ollos, 4 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

comment:9 by Ryan J Ollos, 2 months ago

Milestone: next-stable-1.2.xnext-stable-1.4.x

Modify Ticket

Change Properties
Set your email in Preferences
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.