pg.ddx.io pgsql-sql@postgresql.org mailing list archive
help / color / mirror / Atom feedRace condition in resetting a sequence
4+ messages / 4 participants
[nested] [flat]
* Race condition in resetting a sequence
@ 2007-08-03 20:56 Steve Midgley <public@misuse.org>
0 siblings, 1 reply; 4+ messages in thread
From: Steve Midgley @ 2007-08-03 20:56 UTC (permalink / raw)
To: Scott Marlowe <scott.marlowe@gmail.com>; +Cc: pgsql-sql
Hi Scott,
You've moved into more general territory, so I'm starting a new thread.
The code I provided to reset a primary key sequence is actually part of
Ruby on Rails core library - actually they use something very similar
to what I originally sent:
SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT
increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence}))
FROM #{table}), false)
Where:
#{sequence} = sequence name
#{pk} = primary key of table under sequence
#{table} = table under sequence
Their code is a little different from what I provided before b/c it
increments by one (times the increment #) above the max(pk). But
essentially it's the same. (I think their method leaves small gaps in
the sequence every time it runs). Also I think they're method is likely
to be a little slower (one extra select statement) and therefore
(perhaps) more vulnerable to a race?
You mentioned something more general though: "As long as you're using
setval you have a race condition"? However the postgres manual states:
>The sequence functions, listed in
><http://www.postgresql.org/docs/8.2/interactive/functions-sequence.html#FUNCTIONS-SEQUENCE-TABLE>T...
>9-34, provide simple, multiuser-safe methods for obtaining successive
>sequence values from sequence objects.
(http://www.postgresql.org/docs/8.2/interactive/functions-sequence.html)
Included in Table 9-34 is "setval" - so I'm not clear how it can have a
race condition all by itself? Or maybe it only has a race condition
when used in ways similar to how Ruby/Rails is using it? (i.e. with a
compound select/coalesce statement as one of its parameters?) Would
this command have a race condition:
select setval('my_pk_seq', 500)
This issue is reasonably important since Ruby on Rails is fairly widely
used. As you say, the race window would be pretty small on a compound
select -- and the Ruby function doesn't actually get called very often,
but if you wouldn't mind explaining how the race condition would
manifest, I'll post a warning on the RoR bug tracking site so that
people can at least understand that there's a potential bug here..
Thanks again,
Steve
At 08:42 PM 8/3/2007, Scott Marlowe wrote:
>On 8/3/07, Steve Midgley <public@misuse.org> wrote:
> > Hi Scott,
> >
> > Thanks for this info (and Michael too!).
> >
> > Let me see if I understand your suggestion. I would run these three
> > commands in sequence:
> >
> > # select nextval('[my_seq_name]');
> > returns => 52 [I believe that the sequence is at 52]
> > # alter sequence [my_seq_name] increment by 5000;
> > # select nextval('[my_seq_name]');
> > returns => 5052
> >
> > If the third command doesn't return "5052" - i.e. it returns 5053,
> then
> > I know that somewhere in this sequence another process grabbed an
> id
> > out from under me. It doesn't matter where, but I would know that
> my
> > 5000 id's are not unique and should be discarded? If the third
> command
> > DOES return 5052, then I know that those 5000 id's are "locked" for
> my
> > use and no other application could have grabbed one of them?
>
>No, that's not what would happen. If someone grabbed an id after the
>increment value was changed, then you'd get 10052, cause they would
>increment the sequence by 5,000.since you're not using setval, and
>you're keeping the increment positive, there's no danger of collision,
>only of over-incrementing and leaving a giant hole in your sequence.
>which is ok.
>
> > Can anyone see a flaw in that? It looks right to me..
> >
> > Scott - it also seems to me that I need not waste all those id's if
> > another application does grab one during my statement: If I detect
> a
> > failure, I could just reset the pk sequence back to the max id of
> the
> > underlying table before trying again. I think this code would do it
> > (stolen from Ruby's postgres adaptor):
>
>That is open to a race condition. The bad kind.
>
> > SELECT setval('[seq_name]', (SELECT COALESCE(MAX([pk_of_seq]),
> (SELECT
> > min_value FROM [seq_name])) FROM [table_of_pk]), false)
>
>As long as you're using setval, you have a race condition. Please
>avoid it. Unless you can guarantee that no one else is using the
>database at the same time (during a maintenance window etc...)
>
> > So for table "property" with pk of "id" and sequence name
> > "property_id_seq":
> >
> > SELECT setval('property_id_seq', (SELECT COALESCE(MAX(id), (SELECT
> > min_value FROM property_id_seq)) FROM property), false)
>
>You'd think that the select coalesce and the outer select setval would
>not have a race condition, but they still do. Just a much smaller
>one.
>
> > I'm now starting to think that there's no way to solve this problem
> in
> > an "elegant manner" even in a stored procedure? Your method seems
> to be
> > as good as it's going to get? (Not that I'm complaining!)
>
>Yep. Safe is better than pretty or elegant. :)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Race condition in resetting a sequence
@ 2007-08-04 00:01 Tom Lane <tgl@sss.pgh.pa.us>
parent: Steve Midgley <public@misuse.org>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2007-08-04 00:01 UTC (permalink / raw)
To: Steve Midgley <public@misuse.org>; +Cc: Scott Marlowe <scott.marlowe@gmail.com>; pgsql-sql
Steve Midgley <public@misuse.org> writes:
> The code I provided to reset a primary key sequence is actually part of
> Ruby on Rails core library - actually they use something very similar
> to what I originally sent:
> SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT
> increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence}))
> FROM #{table}), false)
Ugh. That's completely unsafe/broken, unless they also use locking that
you didn't show.
> You mentioned something more general though: "As long as you're using
> setval you have a race condition"? However the postgres manual states:
>> The sequence functions, listed in
>> <http://www.postgresql.org/docs/8.2/interactive/functions-sequence.html#FUNCTIONS-SEQUENCE-TABLE>T...
>> 9-34, provide simple, multiuser-safe methods for obtaining successive
>> sequence values from sequence objects.
> (http://www.postgresql.org/docs/8.2/interactive/functions-sequence.html)
> Included in Table 9-34 is "setval" - so I'm not clear how it can have a
> race condition all by itself?
It doesn't have a race condition "all by itself": it will do what it's
told. The problem with commands such as the above is that there's a
time window between calculating the max() and executing the setval(),
and that window is more than large enough to allow someone else to
insert a row that invalidates your max() computation. (Because of MVCC
snapshotting, the risk window is in fact as long as the entire
calculation of the max --- it's not just a few instructions as some
might naively think.)
Now it is possible to make this brute-force approach safe: you can lock
the table against all other modifications until you've applied your own
changes. But you pay a high price in loss of concurrency if you do
that.
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Race condition in resetting a sequence
@ 2007-08-04 19:33 Lew <lew@lewscanon.nospam>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Lew @ 2007-08-04 19:33 UTC (permalink / raw)
To: pgsql-sql
Steve Midgley writes:
>> The code I provided to reset a primary key sequence is actually part of
>> Ruby on Rails core library - actually they use something very similar
>> to what I originally sent:
...
>> SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT
>> increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence}))
>> FROM #{table}), false)
Tom Lane wrote:
> Ugh. That's completely unsafe/broken, unless they also use locking that
> you didn't show.
...
> It doesn't have a race condition "all by itself": it will do what it's
> told. The problem with commands such as the above is that there's a
> time window between calculating the max() and executing the setval(),
> and that window is more than large enough to allow someone else to
> insert a row that invalidates your max() computation. (Because of MVCC
> snapshotting, the risk window is in fact as long as the entire
> calculation of the max --- it's not just a few instructions as some
> might naively think.)
>
> Now it is possible to make this brute-force approach safe: you can lock
> the table against all other modifications until you've applied your own
> changes. But you pay a high price in loss of concurrency if you do
> that.
All this trouble over semantically-significant ID columns seems to support the
camp that excoriates use of artificial ID columns and autoincrementation
altogether.
The usual argument in their favor is that they speed up performance, but this
epicyclic dance to accomodate FK references to autoincremented keys makes the
case that there is also a performance penalty, and in the more critical
performance area of code development and correctness than in the less critical
search speed area.
--
Lew
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Race condition in resetting a sequence
@ 2007-08-10 21:00 Scott Marlowe <scott.marlowe@gmail.com>
parent: Lew <lew@lewscanon.nospam>
0 siblings, 0 replies; 4+ messages in thread
From: Scott Marlowe @ 2007-08-10 21:00 UTC (permalink / raw)
To: Lew <lew@lewscanon.nospam>; +Cc: pgsql-sql
On 8/4/07, Lew <lew@lewscanon.nospam> wrote:
> Steve Midgley writes:
> >> The code I provided to reset a primary key sequence is actually part of
> >> Ruby on Rails core library - actually they use something very similar
> >> to what I originally sent:
> ...
> >> SELECT setval('#{sequence}', (SELECT COALESCE(MAX(#{pk})+(SELECT
> >> increment_by FROM #{sequence}), (SELECT min_value FROM #{sequence}))
> >> FROM #{table}), false)
>
> Tom Lane wrote:
> > Ugh. That's completely unsafe/broken, unless they also use locking that
> > you didn't show.
> ...
> > It doesn't have a race condition "all by itself": it will do what it's
> > told. The problem with commands such as the above is that there's a
> > time window between calculating the max() and executing the setval(),
> > and that window is more than large enough to allow someone else to
> > insert a row that invalidates your max() computation. (Because of MVCC
> > snapshotting, the risk window is in fact as long as the entire
> > calculation of the max --- it's not just a few instructions as some
> > might naively think.)
> >
> > Now it is possible to make this brute-force approach safe: you can lock
> > the table against all other modifications until you've applied your own
> > changes. But you pay a high price in loss of concurrency if you do
> > that.
>
> All this trouble over semantically-significant ID columns seems to support the
> camp that excoriates use of artificial ID columns and autoincrementation
> altogether.
I don't that it does that at all. If properly implemented, sequences
provide a race free way to assign unique ids to a table that otherwise
would have no natural primary key. I think that having a religious
preference one way or the other means you might use a suboptimal
method in some cases. In the db I work on, we have artificial pks of
sequences, artificial pks made from a large list of pre-created ids (6
alphanum char locator codes, common in the travel industry) and still
other tables that have natural primary keys. Each has a reason for
being the way it is.
I can't see this saying natural pks, which are often open to
interpretation and changes over time are any less are any better than
artificial pks from a sequence at all. I can see it saying that the
people who developed RoR framework were NOT database specialists.
> The usual argument in their favor is that they speed up performance, but this
> epicyclic dance to accomodate FK references to autoincremented keys makes the
> case that there is also a performance penalty, and in the more critical
> performance area of code development and correctness than in the less critical
> search speed area.
Sorry, but that's only one of the two arguments. The other one is
that natural pks are often impossible due to the dataset not being
able to be unique because of the business constraints.
In an airline reservation system, you might start with last and first
name for a pk. Then add phone number. except some people don't wanna
give their phone numbers, so you use steet address, only to find out
that John Smith with no phone lives on 123 Acorn St in Portland while
another John Smith with no phone lives on 123 Acorn St in
Phillidelphia.
And what do you do when they call back about their ticket?
Seriously, the natural primary key method has as many problems as the
artificial one.
In the USAF, they used to use a system to keep track of your medical
records. The first letter of your last name, followed by the last
four of your SSN. So, Ken Johnson might have an SSN of 123-45-6789
While Patricia Jackson might have an SSN of 453-89-6789.
Imagine Ken's surprise when his doctor tells him he's pregnant. Or
Patricia's surprise to have a limb amputated when she went in for a
problem with heart murmers.
By the time you add enough fields together to get a reliable primary
key, you might have a multi-dimensional monster that kills
performance.
You can't just religiously say that one or the other is the only answer.
Finally the cost of software development is only a small part of the
overall cost of creating and deploying a piece of code. Last place I
worked had a transactional database developed really quickly by non-db
people that used isam tables (no, not mysql with myisam, just isam)
that corrupted records dozens of times a day and required a team of
support people to keep it running semi-smoothly and hours of downtime
each night to be cleaned up. The initial savings on development costs
were eaten up rather quickly by the ongoing support costs.
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2007-08-10 21:00 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2007-08-03 20:56 Race condition in resetting a sequence Steve Midgley <public@misuse.org>
2007-08-04 00:01 ` Tom Lane <tgl@sss.pgh.pa.us>
2007-08-04 19:33 ` Lew <lew@lewscanon.nospam>
2007-08-10 21:00 ` Scott Marlowe <scott.marlowe@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox