Re: [BUGS] BUG #7722: extract(epoch from age(...)) appears to be broken

2012-12-02 Thread Alvaro Herrera
aanisi...@inbox.ru wrote:
> The following bug has been logged on the website:
> 
> Bug reference:  7722
> Logged by:  Artem Anisimov
> Email address:  aanisi...@inbox.ru
> PostgreSQL version: 9.2.1
> Operating system:   Slackware Linux 14.0/amd64
> Description:
> 
> The following to queries give the same result (first arguments to age()
> differ in the day number only, second arguments are identical):
> 
> select extract(epoch from age('2012-11-23 16:41:31', '2012-10-23
> 15:56:10'));
> 
> and
> 
> select extract(epoch from age('2012-11-22 16:41:31', '2012-10-23
> 15:56:10'));

alvherre=# select age('2012-11-22 16:41:31', '2012-10-23 15:56:10');
   age
--
 30 days 00:45:21
(1 fila)

alvherre=# select age('2012-11-23 16:41:31', '2012-10-23 15:56:10');
  age   

 1 mon 00:45:21
(1 fila)

The problem is that age() returns 30 days in one case, and "one month" in the
other; extract() then considers the month as equivalent to 30 days.  This is
documented as such, see [1].

[1] http://www.postgresql.org/docs/current/static/functions-datetime.html

I think if you want a precise computation you should just subtract the two
dates and then extract epoch from the result.

alvherre=# select extract(epoch from timestamp '2012-11-22 16:41:31' - 
'2012-10-23 15:56:10');
 date_part 
---
   2594721
(1 fila)

alvherre=# select extract(epoch from timestamp '2012-11-23 16:41:31' - 
'2012-10-23 15:56:10');
 date_part 
---
   2681121
(1 fila)


-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


[BUGS] BUG #7722: extract(epoch from age(...)) appears to be broken

2012-12-02 Thread aanisimov
The following bug has been logged on the website:

Bug reference:  7722
Logged by:  Artem Anisimov
Email address:  aanisi...@inbox.ru
PostgreSQL version: 9.2.1
Operating system:   Slackware Linux 14.0/amd64
Description:

The following to queries give the same result (first arguments to age()
differ in the day number only, second arguments are identical):

select extract(epoch from age('2012-11-23 16:41:31', '2012-10-23
15:56:10'));

and

select extract(epoch from age('2012-11-22 16:41:31', '2012-10-23
15:56:10'));

The problem can also be reproduced in pgsql 9.1.4 of Fedora 17.




-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] PITR potentially broken in 9.2

2012-12-02 Thread Tom Lane
Jeff Janes  writes:
> On Sat, Dec 1, 2012 at 1:56 PM, Tom Lane  wrote:
>> I'm confused.  Are you now saying that this problem only exists in
>> 9.1.x?  I tested current HEAD because you indicated the problem was
>> still there.

> No, I'm saying the problem exists both in 9.1.x and in hypothetical
> 9.2.2 and in hypothetical 9.3, but not in 9.2.[01] because in those it
> is masked by that other problem which has just been fixed.

I'm still confused.  I've now tried this in both HEAD and 9.1 branch
tip, and I do not see any misbehavior.  If I set recovery_target_time to
before the pg_stop_backup time, I get "FATAL:  requested recovery stop
point is before consistent recovery point" which is what I expect; and
if I set it to after the pg_stop_backup time, it starts up as expected.
So if there's a remaining unfixed bug here, I don't understand what
that is.

regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Tom Lane
Simon Riggs  writes:
> On 2 December 2012 16:44, Tom Lane  wrote:
>> As far as the concern about new bugs is concerned: if we start replay
>> from this checkpoint, we will certainly not consider that the DB is
>> consistent until we reach the checkpoint's physical position.  And by
>> that point we'll have replayed the XLOG_RUNNING_XACTS record emitted by
>> LogStandbySnapshot, so our idea of the nextXid should be fully up to
>> date anyway.  The same goes for checkpoints encountered later in the
>> replay run --- they'd just be duplicative of the preceding
>> XLOG_RUNNING_XACTS record.  There is no reason to put the same XID into
>> the checkpoint record.

> Exactly, the end result is identical, but the intermediate states may differ.

Indeed, and the intermediate states are *wrong*, as things stand.  But
they won't be visible to backends AFAICS, since we aren't allowing any
backends in yet.  I'm more concerned about the possible effects on tools
that scan WAL --- and in that connection, it's not apparent to me that
your "minimal fix" is any safer than the other change.  It's a
behavioral change either way.  I think correct tools shouldn't have a
problem either way, but it seems to me to be real hard to argue that
buggy tools would be affected by one change but not the other.  More
likely it could go either way depending on the nature of the bug.

regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Simon Riggs
On 2 December 2012 16:44, Tom Lane  wrote:
> Simon Riggs  writes:
>> On 2 December 2012 15:25, Tom Lane  wrote:
>>> This coding was ill-considered from the word go.
>
>> Agreed, but then I don't have a clear reason why it is that way and
>> yet I'm sure I did it for some reason.
>
> I think you just did it because it looked easy, and you didn't think
> very hard about the consequences.

I don't typically think such thoughts. Your ability to see more deeply
into certain topics is a credit to you, not an implication of laziness
on me.

I'm sure I believed it a necessary or useful thing to do.

> As far as the concern about new bugs is concerned: if we start replay
> from this checkpoint, we will certainly not consider that the DB is
> consistent until we reach the checkpoint's physical position.  And by
> that point we'll have replayed the XLOG_RUNNING_XACTS record emitted by
> LogStandbySnapshot, so our idea of the nextXid should be fully up to
> date anyway.  The same goes for checkpoints encountered later in the
> replay run --- they'd just be duplicative of the preceding
> XLOG_RUNNING_XACTS record.  There is no reason to put the same XID into
> the checkpoint record.

Exactly, the end result is identical, but the intermediate states may differ.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Tom Lane
Simon Riggs  writes:
> On 2 December 2012 15:25, Tom Lane  wrote:
>> This coding was ill-considered from the word go.

> Agreed, but then I don't have a clear reason why it is that way and
> yet I'm sure I did it for some reason.

I think you just did it because it looked easy, and you didn't think
very hard about the consequences.

As far as the concern about new bugs is concerned: if we start replay
from this checkpoint, we will certainly not consider that the DB is
consistent until we reach the checkpoint's physical position.  And by
that point we'll have replayed the XLOG_RUNNING_XACTS record emitted by
LogStandbySnapshot, so our idea of the nextXid should be fully up to
date anyway.  The same goes for checkpoints encountered later in the
replay run --- they'd just be duplicative of the preceding
XLOG_RUNNING_XACTS record.  There is no reason to put the same XID into
the checkpoint record.

regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Simon Riggs
On 2 December 2012 15:25, Tom Lane  wrote:

> I can point right now to one misbehavior this causes: if you run a
> point-in-time recovery with a stop point somewhere in the middle of the
> checkpoint, you should end up with a nextXid corresponding to the stop
> point.  This hack in LogStandbySnapshot causes you to end up with a
> much later nextXid, if you were running hot-standby.

True, though that does not cause any problem.

>> Others may wish to go further, overriding my patches, as they choose.
>
> Okay, I will take the responsibility for changing this, but it needs to
> change.

OK. At least we have the minimal coding to fall back on if need be.

> This coding was ill-considered from the word go.

Agreed, but then I don't have a clear reason why it is that way and
yet I'm sure I did it for some reason.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Tom Lane
Simon Riggs  writes:
> I've applied an absolutely minimal fix on this, which introduces no
> other changes that could cause unforeseen consequences.

This is not what we'd agreed to do, I thought.

Now that I've thought more about this bug, the existing coding is flat
out wrong, with or without correction of the epoch.  As you yourself
just wrote in a comment, the checkpoint record logically belongs to the
"redo" point in the WAL stream, not to where it's physically located.
Having it carry a nextXid that belongs to the later point is simply
wrong.  Having it carry different nextXids depending on wal_level is
even more wrong.

I can point right now to one misbehavior this causes: if you run a
point-in-time recovery with a stop point somewhere in the middle of the
checkpoint, you should end up with a nextXid corresponding to the stop
point.  This hack in LogStandbySnapshot causes you to end up with a
much later nextXid, if you were running hot-standby.

> Others may wish to go further, overriding my patches, as they choose.

Okay, I will take the responsibility for changing this, but it needs to
change.  This coding was ill-considered from the word go.

regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Simon Riggs
On 2 December 2012 12:51, Simon Riggs  wrote:
> On 1 December 2012 22:56, Tom Lane  wrote:
>> tar...@gmail.com writes:
>>> [ txid_current can show a bogus value near XID wraparound ]
>>> This happens only if wal_level=hot_standby.
>>
>> I believe what is happening here is
>>
>> (1) CreateCheckPoint sets up checkPoint.nextXid and
>> checkPoint.nextXidEpoch, near xlog.c line 7070 in HEAD.  At this point,
>> nextXid is still a bit less than the wrap point.
>>
>> (2) After performing the checkpoint, at line 7113, CreateCheckPoint
>> calls LogStandbySnapshot() which "helpfully" updates checkPoint.nextXid
>> to the latest value.  Which by now has wrapped around.  But it doesn't
>> fix checkPoint.nextXidEpoch, so the checkpoint that gets written out has
>> effectively lost the epoch bump that should have happened.
>>
>> While we could add some more logic to try to correct the epoch value
>> in this scenario, I think it's a much better idea to just stop having
>> LogStandbySnapshot update the nextXid.  That seems to me to be useless
>> complication.  I also quite dislike the fact that we're effectively
>> redefining the checkpoint nextXid from being taken before the main
>> body of the checkpoint to being taken afterwards, but *only* in
>> XLogStandbyInfoActive mode.  If that inconsistency isn't already causing
>> bugs (besides this one) today, it'll probably cause them in the future.
>
> I agree that the coding looks weird and agree it shouldn't be there.
> The meaning of the checkpoint values should not differ because
> wal_level has changed.
>
>> So barring objections, I'm going to remove LogStandbySnapshot's behavior
>> of returning the updated nextXid.
>
> Removing it may cause other bugs, but if so, those other bugs need to
> be solved in the right way, not by having a too-far-forwards nextxid
> on the checkpoint record. Having said that, I can't see any bugs that
> would be caused by this.

Andres'  patch is invasive and is not for my liking in backbranches.

Tom's recommended change goes further still and not one I'm
comfortable risking, even though I agree with the root cause/change.

I've applied an absolutely minimal fix on this, which introduces no
other changes that could cause unforeseen consequences.

Others may wish to go further, overriding my patches, as they choose.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Simon Riggs
On 1 December 2012 22:56, Tom Lane  wrote:
> tar...@gmail.com writes:
>> [ txid_current can show a bogus value near XID wraparound ]
>> This happens only if wal_level=hot_standby.
>
> I believe what is happening here is
>
> (1) CreateCheckPoint sets up checkPoint.nextXid and
> checkPoint.nextXidEpoch, near xlog.c line 7070 in HEAD.  At this point,
> nextXid is still a bit less than the wrap point.
>
> (2) After performing the checkpoint, at line 7113, CreateCheckPoint
> calls LogStandbySnapshot() which "helpfully" updates checkPoint.nextXid
> to the latest value.  Which by now has wrapped around.  But it doesn't
> fix checkPoint.nextXidEpoch, so the checkpoint that gets written out has
> effectively lost the epoch bump that should have happened.
>
> While we could add some more logic to try to correct the epoch value
> in this scenario, I think it's a much better idea to just stop having
> LogStandbySnapshot update the nextXid.  That seems to me to be useless
> complication.  I also quite dislike the fact that we're effectively
> redefining the checkpoint nextXid from being taken before the main
> body of the checkpoint to being taken afterwards, but *only* in
> XLogStandbyInfoActive mode.  If that inconsistency isn't already causing
> bugs (besides this one) today, it'll probably cause them in the future.

I agree that the coding looks weird and agree it shouldn't be there.
The meaning of the checkpoint values should not differ because
wal_level has changed.

> So barring objections, I'm going to remove LogStandbySnapshot's behavior
> of returning the updated nextXid.

Removing it may cause other bugs, but if so, those other bugs need to
be solved in the right way, not by having a too-far-forwards nextxid
on the checkpoint record. Having said that, I can't see any bugs that
would be caused by this.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs


Re: [BUGS] BUG #7710: Xid epoch is not updated properly during checkpoint

2012-12-02 Thread Simon Riggs
On 1 December 2012 23:10, Andres Freund  wrote:

>> So barring objections, I'm going to remove LogStandbySnapshot's behavior
>> of returning the updated nextXid.
>
> I don't see any reason why it would be bad to remove this. I think the
> current behaviour could actually even delay getting to an active state
> slightly in the presence of prepared transactions because its used to
> create to initialize the KnownAssignedXid machinery in xlog_redo. If the
> prepared xacts are suboverflown its a *good* thing to have an old
> ->nextXid.

That particular coding was pretty weird. Please change.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs