set_public_id can raise IntegrityError when two annotations are added concurrently

wjt commented

2015-05-19 16:44:50 +00:00

If more than one annotation is added to the same scene concurrently, the update() call in Annotation.set_public_id can throw IntegrityError: duplicate key value violates unique constraint "annotation_annotation_public_id_key", because there is a race between determining the current highest annotation ID for a scene, and setting that+1 on the new annotation.

It's a bit cheesy, but assuming Annotation.id starts from 0 and increases by 1 for each new annotation, I think it would be safe to just define:

    self.public_id = "%s/%s" % (self.item.public_id, ox.toAZ(self.id + 1))

Another option would be to create a new SEQUENCE starting at max(public_id) + 1 at the time of the upgrade and use that. Both of these make the 'local' part of the public_id globally unique but surely nothing is relying on the first annotation on item XYZ being XYZ/A?

(This isn't hypothetical, users are seeing this error in practice.)

If more than one annotation is added to the same scene concurrently, the `update()` call in [Annotation.set_public_id](https://wiki.0x2620.org/browser/pandora/pandora/annotation/models.py#L116) can throw `IntegrityError: duplicate key value violates unique constraint "annotation_annotation_public_id_key"`, because there is a race between determining the current highest annotation ID for a scene, and setting that+1 on the new annotation. It's a bit cheesy, but assuming `Annotation.id` starts from `0` and increases by `1` for each new annotation, I think it would be safe to just define: ``` self.public_id = "%s/%s" % (self.item.public_id, ox.toAZ(self.id + 1)) ``` Another option would be to create a new `SEQUENCE` starting at `max(public_id) + 1` at the time of the upgrade and use that. Both of these make the 'local' part of the `public_id` globally unique but surely nothing is relying on the first annotation on item `XYZ` being `XYZ/A`? (This isn't hypothetical, users are seeing this error in practice.)

j added the

backend

label 2015-05-19 16:44:50 +00:00

j added this to the 14.04 milestone 2015-05-19 16:44:50 +00:00

j self-assigned this 2015-05-19 16:44:50 +00:00

j added the

normal

defect

labels 2015-05-19 16:44:50 +00:00

j commented

2015-05-19 17:16:49 +00:00

Owner

We really want to start with /A for the first annotation per item to keep urls short.

I have not seen this integrity error in a long time. Is there unusually high load at your instance?

creating a sequence per item seams to be the best option to me.

We really want to start with <ID>/A for the first annotation per item to keep urls short. I have not seen this integrity error in a long time. Is there unusually high load at your instance? creating a sequence per item seams to be the best option to me.

j commented

2015-05-19 19:10:42 +00:00

Owner

using many sequences in postgres does not perform well, will be using a table instead.

j commented

2015-05-19 19:13:38 +00:00

Owner

In []changeset:pandora,4976:

#!CommitTicketReference repository="" revision="pandora,4976"
track annotation id in table to avoid IntegrityErrors while adding multiple annotations to one item, fixes #2780

In []changeset:pandora,4976: ``` #!CommitTicketReference repository="" revision="pandora,4976" track annotation id in table to avoid IntegrityErrors while adding multiple annotations to one item, fixes #2780 ```

j added the

fixed

label 2015-05-19 19:13:38 +00:00

j closed this issue

2015-05-19 19:13:38 +00:00

wjt commented

2015-05-19 21:20:29 +00:00

Author

Not unusually high load that I could see – it was only happening (regularly) for one user, and I couldn't work out what she was doing differently. Naturally it couldn't be reproduced when I was watching…

Thanks!

Not unusually high load that I could see – it was only happening (regularly) for one user, and I couldn't work out what she was doing differently. Naturally it couldn't be reproduced when I was watching… Thanks!

set_public_id can raise IntegrityError when two annotations are added concurrently #2780