Thread

Re: Bug #613: Sequence values fall back to previously chec

Vadim Mikheev <vmikheev@sectorbase.com> — 2002-03-15T00:17:39Z

> > This isn't an issue for a SELECT nextval() standing on
> > its own AFAIK the result will not be transmitted to the
> > client until after the commit happens. But it would be
> > an issue for a select executed inside a transaction
> > block (begin/commit).
> 
> The behavior of SELECT nextval() should not be conditional
> on being in or out of a transaction block.

And it's not. But behaviour of application *must* be
conditional on was transaction committed or not.

What's the problem for application that need nextval() for
external (out-of-database) purposes to use sequence values
only after transaction commit? What's *wrong* for such application
to behave the same way as when dealing with other database objects
which are under transaction control (eg only after commit you can
report to user that $100 was successfully added to his/her account)?

---

I agree that if nextval-s were only "write" actions in transaction
and they made some XLogInsert-s then WAL must be flushed at commit
time. But that's it. Was this fixed? Very easy.

Vadim

Re: Bug #613: Sequence values fall back to previously chec

Ben Grimm <bgrimm@zaeon.com> — 2002-03-15T01:55:23Z

On Thu, 14 Mar 2002, Mikheev, Vadim wrote:

> And it's not. But behaviour of application *must* be
> conditional on was transaction committed or not.
> 
> What's the problem for application that need nextval() for
> external (out-of-database) purposes to use sequence values
> only after transaction commit? What's *wrong* for such application
> to behave the same way as when dealing with other database objects
> which are under transaction control (eg only after commit you can
> report to user that $100 was successfully added to his/her account)?

But sequences should not be under transaction control.  Can you 
safely rollback a sequence?  No!  The only way to ensure that would
be to lock the sequence for the duration of the transaction.  If 
you want an ACID compliant sequential value, you implement it using
a transaction safe method (e.g. a table with rows you can lock for
the duration of a transaction).  If you want a number that is 
guaranteed to always move in one direction, return the next value
without requiring locks, and may have gaps in the numbers returned,
you choose a sequence.

Placing a restriction on an application that says it must treat 
the values returned from a sequence as if they might not be committed
is absurd.  What about applications that don't use explicit 
transactions?  As soon as a result comes back it should be considered
'live', on disk, never think about it again. 

> I agree that if nextval-s were only "write" actions in transaction
> and they made some XLogInsert-s then WAL must be flushed at commit
> time. But that's it. Was this fixed? Very easy.

But aren't the nextval's always going to be the only write actions
in their transactions since the nextval isn't really a part of the 
transaction that called it?  If it were, then it could be rolled 
back along with that transaction.  This is why you can, right now, 
insert data into a table with a serial column, committing after
each row, crash the database and STILL have the sequence fall back
to its initial state.  The XLogInserts that occur from the table
inserts must not happen in the same xact as the nextval's 
XLogInserts.  I can demonstrate the behavior quite easilly, and 
Bruce posted results that confirmed it.

-- Ben

Re: Bug #613: Sequence values fall back to previously chec

Vadim Mikheev <vmikheev@sectorbase.com> — 2002-03-15T09:05:33Z

> But sequences should not be under transaction control.  Can you
> safely rollback a sequence?  No!  The only way to ensure that would
...
> Placing a restriction on an application that says it must treat the values
> returned from a sequence as if they might not be committed is absurd.

Why? The fact that you are not able to rollback sequences does not
necessary mean that you are not required to perform commit to ensure
permanent storage of changes made to database.

And isn't it absurd to do more XLogFlush-es for non-transactional objects
than we do for transactional ones? And why? Just for convenience of
<< 1% applications which need to use sequences in their own,
non-database, external objects? We are not required to care about those
objects, but we'd better care about performance of our operations over our
objects.

> > I agree that if nextval-s were only "write" actions in transaction
> > and they made some XLogInsert-s then WAL must be flushed at commit
> > time. But that's it. Was this fixed? Very easy.
>
> But aren't the nextval's always going to be the only write actions
> in their transactions since the nextval isn't really a part of the
> transaction that called it?  If it were, then it could be rolled

There are no nextval' transactions. See how XLOG_NO_TRAN flag
is used in XLogInsert and you'll see why there is no XLogFlush
after transaction-with-nextval-only (which causes N1 reported problem).

> back along with that transaction.  This is why you can, right now,
> insert data into a table with a serial column, committing after
> each row, crash the database and STILL have the sequence fall back
> to its initial state.  The XLogInserts that occur from the table
> inserts must not happen in the same xact as the nextval's
> XLogInserts.  I can demonstrate the behavior quite easilly, and
> Bruce posted results that confirmed it.

Just wait until Tom adds check for system RedoRecPtr in nextval()
and try to reproduce this behaviour (N2 reported problem)
after that.

Vadim

Re: Bug #613: Sequence values fall back to previously chec

Ben Grimm <bgrimm@zaeon.com> — 2002-03-15T13:23:58Z

On Fri, 15 Mar 2002, Vadim Mikheev wrote:

> > But sequences should not be under transaction control.  Can you
> > safely rollback a sequence?  No!  The only way to ensure that would
> ...
> > Placing a restriction on an application that says it must treat the values
> > returned from a sequence as if they might not be committed is absurd.
> 
> Why? The fact that you are not able to rollback sequences does not
> necessary mean that you are not required to perform commit to ensure
> permanent storage of changes made to database.

I'm not sure I agree, but I'll wait to see the behavior of the db after
the changes are made.

> And isn't it absurd to do more XLogFlush-es for non-transactional objects
> than we do for transactional ones? And why? Just for convenience of
> << 1% applications which need to use sequences in their own,
> non-database, external objects? We are not required to care about those
> objects, but we'd better care about performance of our operations over our
> objects.

Yes, absolutely - if there's a better way, which apparently there is, 
then sure, eliminate the calls to XLogFlush.  It's a workaround, a hack.
I am much more concerned with getting the behavior correct than I am 
about getting some code with my name on it into a release.  My workarounds 
only served to point out flaws in the design, even if I didn't quite
understand at the time why they worked :-)

> There are no nextval' transactions. See how XLOG_NO_TRAN flag
> is used in XLogInsert and you'll see why there is no XLogFlush
> after transaction-with-nextval-only (which causes N1 reported problem).
> 
> Just wait until Tom adds check for system RedoRecPtr in nextval()
> and try to reproduce this behaviour (N2 reported problem)
> after that.
> 

Thank you!  I think I have much better understanding of how this works 
now. 

When these bugs are fixed there is still the issue of bug #3 that I 
came across.  The one that I work around by resetting log_cnt to 0 when a 
backend initializes a sequence.  It's this third bug that made the other 
two so apparent.  Fixing them does not obviate the need to fix this one.

Is there a way to intercept writes or reads such that when a sequnce is
going to or from disk that we can force log_cnt = 0?  Right now that's 
worked around by my 'reset_logcnt' flag in the patch, but I know that it 
may not be an ideal solution.  But, since sequences are just tuples like 
everything else I don't see an obvious way to prevent it.  

-- Ben

Re: Bug #613: Sequence values fall back to previously chec

Tom Lane <tgl@sss.pgh.pa.us> — 2002-03-15T14:34:36Z

"'Ben Grimm'" <bgrimm@zaeon.com> writes:
> When these bugs are fixed there is still the issue of bug #3 that I 
> came across.  The one that I work around by resetting log_cnt to 0 when a 
> backend initializes a sequence.  It's this third bug that made the other 
> two so apparent.  Fixing them does not obviate the need to fix this one.

What's bug #3?  I don't recall a third issue.

			regards, tom lane

Re: Bug #613: Sequence values fall back to previously chec

Tom Lane <tgl@sss.pgh.pa.us> — 2002-03-15T14:39:04Z

Attached is a patch against current CVS that fixes both of the known
problems with sequences: failure to flush XLOG after a transaction
that only does "SELECT nextval()", and failure to force a new WAL
record to be written on the first nextval after a checkpoint.
(The latter uses Vadim's idea of looking at the sequence page LSN.)
I haven't tested it really extensively, but it seems to cure the
reported problems.

Some notes:

1. I found what I believe is another bug in the sequence logic:
		fetch = log = fetch - log + SEQ_LOG_VALS;
should be
		fetch = log = fetch + SEQ_LOG_VALS;
I can't see any reason to reduce the number of values prefetched
by the number formerly prefetched.  Also, if the sequence's "cache"
setting is large (more than SEQ_LOG_VALS), the original code could
easily fail to fetch as many values as it was supposed to cache,
let alone additional ones to be prefetched and logged.

2. I renamed XLogCtl->RedoRecPtr to SavedRedoRecPtr, and renamed
the associated routines to SetSavedRedoRecPtr/GetSavedRedoRecPtr,
in hopes of reducing confusion.

3. I believe it'd now be possible to remove SavedRedoRecPtr and
SetSavedRedoRecPtr/GetSavedRedoRecPtr entirely, in favor of letting
the postmaster fetch the updated pointer with GetRedoRecPtr just
like a backend would.  This would be cleaner and less code ... but
someone might object that it introduces a risk of postmaster hangup,
if some backend crashes whilst holding info_lck.  I consider that
risk minuscule given the short intervals in which info_lck is held,
but it can't be denied that the risk is not zero.  Thoughts?

Comments?  Unless I hear objections I will patch this in current
and the 7.2 branch.  (If we agree to remove SavedRedoRecPtr,
though, I don't think we should back-patch that change.)

			regards, tom lane

Re: [HACKERS] Bug #613: Sequence values fall back to previously

Bruce Momjian <pgman@candle.pha.pa.us> — 2002-03-15T15:43:25Z

Tom Lane wrote:
> Attached is a patch against current CVS that fixes both of the known
> problems with sequences: failure to flush XLOG after a transaction
> that only does "SELECT nextval()", and failure to force a new WAL
> record to be written on the first nextval after a checkpoint.
> (The latter uses Vadim's idea of looking at the sequence page LSN.)
> I haven't tested it really extensively, but it seems to cure the
> reported problems.

I can confirm that the patch fixes the problem shown in my simple test:

test=> create table test (x serial, y varchar(255));
NOTICE:  CREATE TABLE will create implicit sequence 'test_x_seq' for SERIAL column 'test.x'
NOTICE:  CREATE TABLE / UNIQUE will create implicit index 'test_x_key' for table 'test'
CREATE
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16561 1
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16562 1
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16563 1
...

test=> select nextval('test_x_seq');
 nextval 
---------
      22
(1 row)

test=> checkpoint;
CHECKPOINT
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16582 1
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16583 1
test=> insert into test (y) values ('lkjasdflkja sdfl;kj asdfl;kjasdf');
INSERT 16584 1

[ kill -9 to backend ]

#$ sql test
Welcome to psql, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms
       \h for help with SQL commands
       \? for help on internal slash commands
       \g or terminate with semicolon to execute query
       \q to quit

test=> select nextval('test_x_seq');
 nextval 
---------
      56
(1 row)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [HACKERS] Bug #613: Sequence values fall back to previously

Bruce Momjian <pgman@candle.pha.pa.us> — 2002-03-15T15:44:16Z

Tom Lane wrote:
> 2. I renamed XLogCtl->RedoRecPtr to SavedRedoRecPtr, and renamed
> the associated routines to SetSavedRedoRecPtr/GetSavedRedoRecPtr,
> in hopes of reducing confusion.

Good.

> 3. I believe it'd now be possible to remove SavedRedoRecPtr and
> SetSavedRedoRecPtr/GetSavedRedoRecPtr entirely, in favor of letting
> the postmaster fetch the updated pointer with GetRedoRecPtr just
> like a backend would.  This would be cleaner and less code ... but
> someone might object that it introduces a risk of postmaster hangup,
> if some backend crashes whilst holding info_lck.  I consider that
> risk minuscule given the short intervals in which info_lck is held,
> but it can't be denied that the risk is not zero.  Thoughts?

The change sounds good to me.

> Comments?  Unless I hear objections I will patch this in current
> and the 7.2 branch.  (If we agree to remove SavedRedoRecPtr,
> though, I don't think we should back-patch that change.)

Totally agree.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Bug #613: Sequence values fall back to previously chec

Ben Grimm <bgrimm@zaeon.com> — 2002-03-15T15:44:35Z

On Fri, 15 Mar 2002, Tom Lane wrote:

> "'Ben Grimm'" <bgrimm@zaeon.com> writes:
> > When these bugs are fixed there is still the issue of bug #3 that I 
> > came across.  The one that I work around by resetting log_cnt to 0 when a 
> > backend initializes a sequence.  It's this third bug that made the other 
> > two so apparent.  Fixing them does not obviate the need to fix this one.
> 
> What's bug #3?  I don't recall a third issue.
> 

The problem I was seeing before is that when the postmaster was shutdown 
properly, log_cnt in the sequence record was saved with whatever value it 
had at the time.  So when it loaded from disk it would have a value greater 
than zero resulting in no XLogInsert until you'd exceded log_cnt calls to
nextval.  

AFAICT, your patch fixes this problem, as I can't reproduce it now.  

Thanks!

-- Ben

Re: Bug #613: Sequence values fall back to previously chec

Tom Lane <tgl@sss.pgh.pa.us> — 2002-03-15T16:03:50Z

"'Ben Grimm'" <bgrimm@zaeon.com> writes:
> On Fri, 15 Mar 2002, Tom Lane wrote:
>> What's bug #3?  I don't recall a third issue.

> The problem I was seeing before is that when the postmaster was shutdown 
> properly, log_cnt in the sequence record was saved with whatever value it 
> had at the time.

Right, it's supposed to do that.

> So when it loaded from disk it would have a value greater 
> than zero resulting in no XLogInsert until you'd exceded log_cnt calls to
> nextval.  

This is the same as the post-checkpoint issue: we fix it by forcing an
XLogInsert on the first nextval after a checkpoint (or system startup).

			regards, tom lane

Re: [BUGS] Bug #613: Sequence values fall back to previously chec

Clark C. Evans <cce@clarkevans.com> — 2002-03-16T01:54:02Z

(userland comment)

On Fri, Mar 15, 2002 at 01:05:33AM -0800, Vadim Mikheev wrote:
| > But sequences should not be under transaction control.  Can you
| > safely rollback a sequence?  No!  The only way to ensure that would
| ...
| > Placing a restriction on an application that says it must treat the values
| > returned from a sequence as if they might not be committed is absurd.
| 
| Why? The fact that you are not able to rollback sequences does not
| necessary mean that you are not required to perform commit to ensure
| permanent storage of changes made to database.

I use sequences to generate message identifiers for a simple
external-to-database message passing system.   I also use
them for file upload identifiers.  In both cases, if the
external action (message or file upload) succeeds, I commit; 
otherwise I roll-back.  I assume that the datbase won't give
me a duplicate sequence... otherwise I'd have to find some
other way go get sequences or I'd have duplicate messages
or non-unique file identifiers.

With these changes is this assumption no longer valid?  If
so, this change will break alot of user programs.

| And why? Just for convenience of << 1% applications which need
| to use sequences in their own, non-database, external objects?

I think you may be underestimating the amount of "external resources"
which may be associated with a datbase object.  Regardless, may of the
database features in PostgreSQL are there for 1% or less of the
user base... 

Best,

Clark

-- 
Clark C. Evans                   Axista, Inc.
http://www.axista.com            800.926.5525
XCOLLA Collaborative Project Management Software

Re: [BUGS] Bug #613: Sequence values fall back to previously chec

Rod Taylor <rbt@zort.ca> — 2002-03-16T02:02:25Z

I do basically the same thing for files.  Except I md5 a 4 character
random string, and the sequence ID just incase I get the same one
twice -- as it's never been written in stone that I wouldn't -- not to
mention the high number of requests for returning a sequence ID back
to the pool on a rollback.

Anyway, you might try using the OID rather than a sequence ID but if
you rollback the database commit due to failure of an action
externally, shouldn't you be cleaning up that useless external stuff
as well?
--
Rod Taylor

This message represents the official view of the voices in my head

----- Original Message -----
From: "Clark C . Evans" <cce@clarkevans.com>
To: "Vadim Mikheev" <vmikheev@sectorbase.com>
Cc: <pgsql-hackers@postgresql.org>
Sent: Friday, March 15, 2002 8:54 PM
Subject: Re: [HACKERS] [BUGS] Bug #613: Sequence values fall back to
previously chec


> (userland comment)
>
> On Fri, Mar 15, 2002 at 01:05:33AM -0800, Vadim Mikheev wrote:
> | > But sequences should not be under transaction control.  Can you
> | > safely rollback a sequence?  No!  The only way to ensure that
would
> | ...
> | > Placing a restriction on an application that says it must treat
the values
> | > returned from a sequence as if they might not be committed is
absurd.
> |
> | Why? The fact that you are not able to rollback sequences does not
> | necessary mean that you are not required to perform commit to
ensure
> | permanent storage of changes made to database.
>
> I use sequences to generate message identifiers for a simple
> external-to-database message passing system.   I also use
> them for file upload identifiers.  In both cases, if the
> external action (message or file upload) succeeds, I commit;
> otherwise I roll-back.  I assume that the datbase won't give
> me a duplicate sequence... otherwise I'd have to find some
> other way go get sequences or I'd have duplicate messages
> or non-unique file identifiers.
>
> With these changes is this assumption no longer valid?  If
> so, this change will break alot of user programs.
>
> | And why? Just for convenience of << 1% applications which need
> | to use sequences in their own, non-database, external objects?
>
> I think you may be underestimating the amount of "external
resources"
> which may be associated with a datbase object.  Regardless, may of
the
> database features in PostgreSQL are there for 1% or less of the
> user base...
>
> Best,
>
> Clark
>
> --
> Clark C. Evans                   Axista, Inc.
> http://www.axista.com            800.926.5525
> XCOLLA Collaborative Project Management Software
>
> ---------------------------(end of
broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to
majordomo@postgresql.org
>

Re: [BUGS] Bug #613: Sequence values fall back to previously chec

Vadim Mikheev <vmikheev@sectorbase.com> — 2002-03-16T06:04:09Z

> | > Placing a restriction on an application that says it must treat the
values
> | > returned from a sequence as if they might not be committed is absurd.
> |
> | Why? The fact that you are not able to rollback sequences does not
> | necessary mean that you are not required to perform commit to ensure
> | permanent storage of changes made to database.
>
> I use sequences to generate message identifiers for a simple
> external-to-database message passing system.   I also use
> them for file upload identifiers.  In both cases, if the
> external action (message or file upload) succeeds, I commit;
> otherwise I roll-back.  I assume that the datbase won't give
> me a duplicate sequence... otherwise I'd have to find some

So can you do "select nextval()" in *separate* (committed)
transaction *before* external action and "real" transaction where
you store information (with sequence number) about external
action in database?

BEGIN;
SELECT NEXTVAL();
COMMIT;
BEGIN;
-- Do external actions and store info in DB --
COMMIT/ROLLBACK;

Is this totally unacceptable? Is it really *required* to call nextval()
in *the same* transaction where you store info in DB? Why?

> other way go get sequences or I'd have duplicate messages
> or non-unique file identifiers.
>
> With these changes is this assumption no longer valid?  If

1. It's not valid to assume that sequences will not return duplicate
    numbers if there was no commit after nextval.
2. It doesn't matter when sequence numbers are stored in
   database objects only.
3. But if you're going to use sequence numbers in external objects
    you must (pre)fetch those numbers in separate committed
    transaction.

(Can we have this in FAQ?)

> so, this change will break alot of user programs.
>
> | And why? Just for convenience of << 1% applications which need
> | to use sequences in their own, non-database, external objects?
>
> I think you may be underestimating the amount of "external resources"
> which may be associated with a datbase object.  Regardless, may of the
> database features in PostgreSQL are there for 1% or less of the
> user base...

Please note that I was talking about some *inconvenience*, not about
*inability* of using sequence numbers externally (seems my words were
too short). Above is how to do this. And though I agreed that it's not
very convenient/handy/cosy to *take care* and fetch numbers in
separate committed transaction, but it's required only in those special
cases and I think it's better than do fsync() per each nextval() call what
would affect other users/applications where storing sequence numbers
outside of database is not required.

Vadim