Horton

Thread

  1. age(xid) on hot standby

    Peter Eisentraut <peter_e@gmx.net> — 2011-12-28T18:04:09Z

    The check_postgres txn_wraparound action[0] runs this query:

   SELECT datname, age(datfrozenxid) AS age FROM pg_database WHERE datallowconn ORDER BY 1, 2

On a hot standby, this fails with:

   ERROR:  cannot assign TransactionIds during recovery

So, a couple of things to wonder about:

Is it unreasonable to check for transaction ID wraparound on a standby?
It should mirror the situation on the primary, shouldn't it?

Should the age(xid) function do something more useful on a standby,
e.g., have a custom error message or return null or use the transaction
ID from the master?

The error message is coded as an elog() call, meaning that users
shouldn't see it, but it can evidently be triggered by a user, so maybe
we should decorate it with some detail, depending on the outcome of the
previous question.

(It looks like age(xid) isn't documented at all.  Maybe it should be.)


[0] - http://bucardo.org/check_postgres/check_postgres.pl.html#txn_wraparound

  2. Re: age(xid) on hot standby

    Alvaro Herrera <alvherre@commandprompt.com> — 2011-12-28T18:24:02Z

    Excerpts from Peter Eisentraut's message of mié dic 28 15:04:09 -0300 2011:
> The check_postgres txn_wraparound action[0] runs this query:
> 
>    SELECT datname, age(datfrozenxid) AS age FROM pg_database WHERE datallowconn ORDER BY 1, 2
> 
> On a hot standby, this fails with:
> 
>    ERROR:  cannot assign TransactionIds during recovery
> 
> So, a couple of things to wonder about:
> 
> Is it unreasonable to check for transaction ID wraparound on a standby?
> It should mirror the situation on the primary, shouldn't it?
> 
> Should the age(xid) function do something more useful on a standby,
> e.g., have a custom error message or return null or use the transaction
> ID from the master?

I think we could just have the xid_age call
GetCurrentTransactionIdIfAny, and if that returns InvalidXid, use
ReadNewTransactionId instead.  That xid_age assigns a transaction seems
more of an accident than really intended.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  3. Re: age(xid) on hot standby

    Tom Lane <tgl@sss.pgh.pa.us> — 2011-12-28T19:35:34Z

    Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Peter Eisentraut's message of mi dic 28 15:04:09 -0300 2011:
>> On a hot standby, this fails with:
>> ERROR:  cannot assign TransactionIds during recovery

> I think we could just have the xid_age call
> GetCurrentTransactionIdIfAny, and if that returns InvalidXid, use
> ReadNewTransactionId instead.  That xid_age assigns a transaction seems
> more of an accident than really intended.

The trouble with using ReadNewTransactionId is that it makes the results
volatile, not stable as the function is declared to be.

			regards, tom lane

  4. Re: age(xid) on hot standby

    Peter Eisentraut <peter_e@gmx.net> — 2012-01-15T13:00:03Z

    On ons, 2011-12-28 at 14:35 -0500, Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Excerpts from Peter Eisentraut's message of mié dic 28 15:04:09 -0300 2011:
> >> On a hot standby, this fails with:
> >> ERROR:  cannot assign TransactionIds during recovery
> 
> > I think we could just have the xid_age call
> > GetCurrentTransactionIdIfAny, and if that returns InvalidXid, use
> > ReadNewTransactionId instead.  That xid_age assigns a transaction seems
> > more of an accident than really intended.
> 
> The trouble with using ReadNewTransactionId is that it makes the results
> volatile, not stable as the function is declared to be.

Could we alleviate that problem with some caching within the function?

  5. Re: age(xid) on hot standby

    Alvaro Herrera <alvherre@commandprompt.com> — 2012-01-16T14:44:01Z

    Excerpts from Peter Eisentraut's message of dom ene 15 10:00:03 -0300 2012:
> 
> On ons, 2011-12-28 at 14:35 -0500, Tom Lane wrote:
> > Alvaro Herrera <alvherre@commandprompt.com> writes:
> > > Excerpts from Peter Eisentraut's message of mié dic 28 15:04:09 -0300 2011:
> > >> On a hot standby, this fails with:
> > >> ERROR:  cannot assign TransactionIds during recovery
> > 
> > > I think we could just have the xid_age call
> > > GetCurrentTransactionIdIfAny, and if that returns InvalidXid, use
> > > ReadNewTransactionId instead.  That xid_age assigns a transaction seems
> > > more of an accident than really intended.
> > 
> > The trouble with using ReadNewTransactionId is that it makes the results
> > volatile, not stable as the function is declared to be.
> 
> Could we alleviate that problem with some caching within the function?

Maybe if we have it be invalidated at transaction end, that could work.
So each new transaction would get a fresh value.  If you had a long
running transaction the cached value would get behind, but maybe this is
not a problem or we could design some protection against it.

For the check_postgres case I imagine it opens a new connection on each
round so this would not be a problem.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  6. Re: age(xid) on hot standby

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-16T15:27:03Z

    Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Peter Eisentraut's message of dom ene 15 10:00:03 -0300 2012:
>> On ons, 2011-12-28 at 14:35 -0500, Tom Lane wrote:
>>> The trouble with using ReadNewTransactionId is that it makes the results
>>> volatile, not stable as the function is declared to be.

>> Could we alleviate that problem with some caching within the function?

> Maybe if we have it be invalidated at transaction end, that could work.
> So each new transaction would get a fresh value.

Yeah, I think that would work.

> If you had a long
> running transaction the cached value would get behind, but maybe this is
> not a problem or we could design some protection against it.

Nobody has complained about the fact that age()'s reference point
remains fixed throughout a transaction on the master, so I don't see why
we'd not be happy with that behavior on a standby.

			regards, tom lane

  7. Re: age(xid) on hot standby

    Alvaro Herrera <alvherre@commandprompt.com> — 2012-01-17T01:59:00Z

    Excerpts from Tom Lane's message of lun ene 16 12:27:03 -0300 2012:
> 
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Excerpts from Peter Eisentraut's message of dom ene 15 10:00:03 -0300 2012:
> >> On ons, 2011-12-28 at 14:35 -0500, Tom Lane wrote:
> >>> The trouble with using ReadNewTransactionId is that it makes the results
> >>> volatile, not stable as the function is declared to be.
> 
> >> Could we alleviate that problem with some caching within the function?
> 
> > Maybe if we have it be invalidated at transaction end, that could work.
> > So each new transaction would get a fresh value.
> 
> Yeah, I think that would work.

So who's going to work on a patch?  Peter, are you?  If not, we should
add it to the TODO list.

-- 
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

  8. Re: age(xid) on hot standby

    Peter Eisentraut <peter_e@gmx.net> — 2012-01-18T19:42:45Z

    On mån, 2012-01-16 at 22:59 -0300, Alvaro Herrera wrote:
> Excerpts from Tom Lane's message of lun ene 16 12:27:03 -0300 2012:
> > 
> > Alvaro Herrera <alvherre@commandprompt.com> writes:
> > > Excerpts from Peter Eisentraut's message of dom ene 15 10:00:03 -0300 2012:
> > >> On ons, 2011-12-28 at 14:35 -0500, Tom Lane wrote:
> > >>> The trouble with using ReadNewTransactionId is that it makes the results
> > >>> volatile, not stable as the function is declared to be.
> > 
> > >> Could we alleviate that problem with some caching within the function?
> > 
> > > Maybe if we have it be invalidated at transaction end, that could work.
> > > So each new transaction would get a fresh value.
> > 
> > Yeah, I think that would work.
> 
> So who's going to work on a patch?  Peter, are you?  If not, we should
> add it to the TODO list.

Not at this very moment, but maybe in a few weeks.

  9. Re: age(xid) on hot standby

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-18T19:55:46Z

    Peter Eisentraut <peter_e@gmx.net> writes:
> On mn, 2012-01-16 at 22:59 -0300, Alvaro Herrera wrote:
>> So who's going to work on a patch?  Peter, are you?  If not, we should
>> add it to the TODO list.

> Not at this very moment, but maybe in a few weeks.

BTW, it strikes me that maybe the coding should work about like this:

	if (!TransactionIdIsValid(age_reference_xid))
	{
		age_reference_xid = GetTopTransactionIdIfAny();
		if (!TransactionIdIsValid(age_reference_xid))
			age_reference_xid = ReadNewTransactionId();
	}
	... use age_reference_xid to compute result ...

and of course code somewhere to reset age_reference_xid at end of xact.

The advantage of this is

(1) same code works on master and standby

(2) calling age() no longer requires an otherwise read-only transaction
to acquire an XID.

			regards, tom lane

  10. Re: age(xid) on hot standby

    Simon Riggs <simon@2ndquadrant.com> — 2012-01-18T20:37:31Z

    On Wed, Jan 18, 2012 at 7:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> On mån, 2012-01-16 at 22:59 -0300, Alvaro Herrera wrote:
>>> So who's going to work on a patch?  Peter, are you?  If not, we should
>>> add it to the TODO list.
>
>> Not at this very moment, but maybe in a few weeks.
>
> BTW, it strikes me that maybe the coding should work about like this:
>
>        if (!TransactionIdIsValid(age_reference_xid))
>        {
>                age_reference_xid = GetTopTransactionIdIfAny();
>                if (!TransactionIdIsValid(age_reference_xid))
>                        age_reference_xid = ReadNewTransactionId();
>        }
>        ... use age_reference_xid to compute result ...
>
> and of course code somewhere to reset age_reference_xid at end of xact.
>
> The advantage of this is
>
> (1) same code works on master and standby
>
> (2) calling age() no longer requires an otherwise read-only transaction
> to acquire an XID.

Nice solution.

Also it illustrates that some users write code that tries to do things
on a Hot Standby that are supposed to be illegal.

If the OPs error message had returned the correct SQLCODE then it
would have been better able to handle the message.

I think we should apply the patch to return the correct SQLCODE in all
cases, even if its supposedly not possible.

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

  11. Re: age(xid) on hot standby

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-01-19T03:56:52Z

    Simon Riggs <simon@2ndQuadrant.com> writes:
> I think we should apply the patch to return the correct SQLCODE in all
> cases, even if its supposedly not possible.

[ shrug... ]  My opinion about that has not changed.  Those are internal
sanity checks, and as such, ERRCODE_INTERNAL_ERROR is exactly the right
thing for them.  If there are paths that can reach that code, we need to
find them and plug the holes with appropriate user-facing error checks
that say what it is the user is not supposed to do.  In this example,
if we had decided that the right answer should be for age() to not be
allowed on standbys, then an error saying exactly that would be an
appropriate user-facing error.  "You're not supposed to acquire a
transaction ID" is not intelligible to the average user, and giving it
another error code doesn't improve that situation.

			regards, tom lane

  12. Re: age(xid) on hot standby

    Peter Eisentraut <peter_e@gmx.net> — 2012-05-08T19:01:01Z

    On ons, 2012-01-18 at 14:55 -0500, Tom Lane wrote:
> BTW, it strikes me that maybe the coding should work about like this:
> 
> 	if (!TransactionIdIsValid(age_reference_xid))
> 	{
> 		age_reference_xid = GetTopTransactionIdIfAny();
> 		if (!TransactionIdIsValid(age_reference_xid))
> 			age_reference_xid = ReadNewTransactionId();
> 	}
> 	... use age_reference_xid to compute result ...
> 
> and of course code somewhere to reset age_reference_xid at end of xact.

How about this patch?

  13. Re: age(xid) on hot standby

    Simon Riggs <simon@2ndquadrant.com> — 2012-05-08T23:55:04Z

    On 8 May 2012 20:01, Peter Eisentraut <peter_e@gmx.net> wrote:
> On ons, 2012-01-18 at 14:55 -0500, Tom Lane wrote:
>> BTW, it strikes me that maybe the coding should work about like this:
>>
>>       if (!TransactionIdIsValid(age_reference_xid))
>>       {
>>               age_reference_xid = GetTopTransactionIdIfAny();
>>               if (!TransactionIdIsValid(age_reference_xid))
>>                       age_reference_xid = ReadNewTransactionId();
>>       }
>>       ... use age_reference_xid to compute result ...
>>
>> and of course code somewhere to reset age_reference_xid at end of xact.
>
> How about this patch?

I think we should fix this, but not with this exact patch.

We should just use MyPgXact->xid
rather than add more to the transaction path

I'll simplify the patch and commit.

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

  14. Re: age(xid) on hot standby

    Simon Riggs <simon@2ndquadrant.com> — 2012-05-09T13:04:08Z

    On 9 May 2012 00:55, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 8 May 2012 20:01, Peter Eisentraut <peter_e@gmx.net> wrote:
>> On ons, 2012-01-18 at 14:55 -0500, Tom Lane wrote:
>>> BTW, it strikes me that maybe the coding should work about like this:
>>>
>>>       if (!TransactionIdIsValid(age_reference_xid))
>>>       {
>>>               age_reference_xid = GetTopTransactionIdIfAny();
>>>               if (!TransactionIdIsValid(age_reference_xid))
>>>                       age_reference_xid = ReadNewTransactionId();
>>>       }
>>>       ... use age_reference_xid to compute result ...
>>>
>>> and of course code somewhere to reset age_reference_xid at end of xact.
>>
>> How about this patch?
>
> I think we should fix this, but not with this exact patch.
>
> We should just use MyPgXact->xid
> rather than add more to the transaction path
>
> I'll simplify the patch and commit.

Committed, but forgot to give appropriate credit. Sorry about that.

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

  15. Re: age(xid) on hot standby

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-05-09T14:34:59Z

    Simon Riggs <simon@2ndQuadrant.com> writes:
>> We should just use MyPgXact->xid
>> rather than add more to the transaction path
>> 
>> I'll simplify the patch and commit.

> Committed, but forgot to give appropriate credit. Sorry about that.

This patch didn't fix things, it broke things.  The former guarantee
that age's reference point would hold still throughout a transaction
just disappeared.

What I read your previous suggestion to be was that age() would keep
local state and use inspection of the current VXID to tell if its
cache was stale.  That would keep the fix localized (which I agree
is a good idea) without losing the stability guarantee.

			regards, tom lane

  16. Re: age(xid) on hot standby

    Simon Riggs <simon@2ndquadrant.com> — 2012-05-09T16:13:28Z

    On 9 May 2012 15:34, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
>>> We should just use MyPgXact->xid
>>> rather than add more to the transaction path
>>>
>>> I'll simplify the patch and commit.
>
>> Committed, but forgot to give appropriate credit. Sorry about that.
>
> This patch didn't fix things, it broke things.  The former guarantee
> that age's reference point would hold still throughout a transaction
> just disappeared.
>
> What I read your previous suggestion to be was that age() would keep
> local state and use inspection of the current VXID to tell if its
> cache was stale.  That would keep the fix localized (which I agree
> is a good idea) without losing the stability guarantee.

Gotcha. Will fix.

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