Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 23 January 2017 at 12:34, Craig Ringer wrote: > On 20 January 2017 at 21:40, Alvaro Herrera wrote: > >> One option would be to add another limit Xid which advances before the >> truncation but which is not used for other decisions other than limiting >> what can users consult. > > This could be useful for other things, but it's probably heavier than needed. > > What I've done in the latest revision of the txid_status() patch is > simply to advance OldestXid _before_ truncating the clog. The rest of > the xid info is advanced after. Currently this is incorporated into > the txid_status patch, but can be separated if desired. > > Relevant commit message portion: > > There was previously no way to look up an arbitrary xid without > running the risk of having clog truncated out from under you. This > hasn't been a problem because anything looking up xids in clog knows > they're protected by datminxid, but that's not the case for arbitrary > user-supplied XIDs. clog was truncated before we advance oldestXid so > taking XidGenLock was insufficient. There's no way to look up a > SLRU with soft-failure. To address this, increase oldestXid under > XidGenLock > before we trunate clog rather than after, so concurrent access is safe. > > Note that while oldestXid is advanced before clog truncation, the xid > limits are advanced _after_ it. If we advanced the xid limits before > truncation too, we'd theoretically run the risk of allocating an xid > from the clog section we're about to truncate, which would be no fun. > (In practice it can't really happen since we only use 1/2 the > available space at a time). > > Moving the lower bound up, truncating, and moving the upper bound up > is the way to go IMO. Patch with explanation in commit msg: https://www.postgresql.org/message-id/camsr+yhodeduua5vw1_rjps_assvemejn_34rqd3pzhf5of...@mail.gmail.com -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 20 January 2017 at 21:40, Alvaro Herrera wrote: > One option would be to add another limit Xid which advances before the > truncation but which is not used for other decisions other than limiting > what can users consult. This could be useful for other things, but it's probably heavier than needed. What I've done in the latest revision of the txid_status() patch is simply to advance OldestXid _before_ truncating the clog. The rest of the xid info is advanced after. Currently this is incorporated into the txid_status patch, but can be separated if desired. Relevant commit message portion: There was previously no way to look up an arbitrary xid without running the risk of having clog truncated out from under you. This hasn't been a problem because anything looking up xids in clog knows they're protected by datminxid, but that's not the case for arbitrary user-supplied XIDs. clog was truncated before we advance oldestXid so taking XidGenLock was insufficient. There's no way to look up a SLRU with soft-failure. To address this, increase oldestXid under XidGenLock before we trunate clog rather than after, so concurrent access is safe. Note that while oldestXid is advanced before clog truncation, the xid limits are advanced _after_ it. If we advanced the xid limits before truncation too, we'd theoretically run the risk of allocating an xid from the clog section we're about to truncate, which would be no fun. (In practice it can't really happen since we only use 1/2 the available space at a time). Moving the lower bound up, truncating, and moving the upper bound up is the way to go IMO. > Another option is not to implement direct reads > from the clog. I think there's a pretty decent argument for having clog lookups; txid_status(...) serves as a useful halfway position between accepting indeterminate commit status on connection loss and using full 2PC. > Yet another option is that before we add such interface > somebody produces proof that the problem does not, in fact, exist. It does exist. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 20 January 2017 at 05:32, Peter Eisentraut wrote: > On 1/19/17 10:06 AM, Alvaro Herrera wrote: >>> Also, I wonder whether we should not in vacuum.c change the order of the >>> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just >>> to keep everything consistent. >> >> I am wary of doing that. The current coding is well battle-tested by >> now, but doing things in the opposite order, not at all. Pending some >> testing to show that there is no problem with a change, I would leave >> things as they are. > > I appreciate this hesitation. > > The issue the patch under discussion is addressing is that we have a > user-space interface to read information about commit timestamps. So if > we truncate away old information before we update the mark where the > good information starts, then we get the race. We don't have a > user-space interface reading, say, the clog, but it doesn't seem > implausible for that to exist at some point. How would this code have > to be structured then? I have a patch in the current commitfest that exposes just such a user interface, txid_status() . See https://commitfest.postgresql.org/12/730/ . Robert was about to commit when he identified this race in commit status lookup, which led me to identify the same race addressed here for commit timestamps. >> What I fear is: what >> happens if a concurrent checkpoint reads the values between the two >> operations, and a crash occurs? I think that the checkpoint might save >> the updated values, so after crash recovery the truncate would not be >> executed, possibly leaving files around. Leaving files around might be >> dangerous for multixacts at least (it's probably harmless for xids). > > Why is leaving files around dangerous? As far as I can tell, leaving the files around as such isn't dangerous. The problem arises if we wrap around and start treating old SLRU contents as new again without first truncating them away. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
Peter Eisentraut wrote: > On 1/19/17 10:06 AM, Alvaro Herrera wrote: > >> Also, I wonder whether we should not in vacuum.c change the order of the > >> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just > >> to keep everything consistent. > > > > I am wary of doing that. The current coding is well battle-tested by > > now, but doing things in the opposite order, not at all. Pending some > > testing to show that there is no problem with a change, I would leave > > things as they are. > > I appreciate this hesitation. > > The issue the patch under discussion is addressing is that we have a > user-space interface to read information about commit timestamps. So if > we truncate away old information before we update the mark where the > good information starts, then we get the race. Correct. > We don't have a user-space interface reading, say, the clog, but it > doesn't seem implausible for that to exist at some point. How would > this code have to be structured then? One option would be to add another limit Xid which advances before the truncation but which is not used for other decisions other than limiting what can users consult. Another option is not to implement direct reads from the clog. Yet another option is that before we add such interface somebody produces proof that the problem does not, in fact, exist. I think producing proof is onerous enough that nobody will step up to it until there is some real need for user-invoked clog lookups. (We've gone so long without them, perhaps we will never need them.) > > What I fear is: what > > happens if a concurrent checkpoint reads the values between the two > > operations, and a crash occurs? I think that the checkpoint might save > > the updated values, so after crash recovery the truncate would not be > > executed, possibly leaving files around. Leaving files around might be > > dangerous for multixacts at least (it's probably harmless for xids). > > Why is leaving files around dangerous? Leftover files could confuse the truncation algorithm. We already had this problem for multixacts, and it took a lot of sweat and blood to get it fixed. The algorithm has since been fixed, but the mechanism is delicate enough that I am afraid that tinkering with it may break it. > If this is a problem, why is it not a problem for commit timestamps? > I don't understand why these different SLRU uses are different. commit_ts heavily depends on the clog cycle, so my thinking is that if clog is protected enough, then so is commit_ts. Given a strong clog, I expect commit_ts not to break. If I make clog brittle, will that break commit_ts too? Perhaps. Maybe not, but again that would require more study. I have looked at clog a bit more this morning and I think perhaps it is safe to make it work in the same way as commit_ts -- but somebody would have to go to the trouble of verifying that it is. > Yeah, we could go ahead with this patch as is and it might be fine, but > I feel like we don't understand the broader issue completely yet. ISTM the more complicated uses of slru.c have turned out to have a lot of emergent properties that weren't completely understood, so yeah I agree that we don't (or at least I don't, and most people don't either). Someday, somebody will rewrite slru.c and/or remove multixacts, and these problems will all go away. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 1/19/17 10:06 AM, Alvaro Herrera wrote: >> Also, I wonder whether we should not in vacuum.c change the order of the >> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just >> to keep everything consistent. > > I am wary of doing that. The current coding is well battle-tested by > now, but doing things in the opposite order, not at all. Pending some > testing to show that there is no problem with a change, I would leave > things as they are. I appreciate this hesitation. The issue the patch under discussion is addressing is that we have a user-space interface to read information about commit timestamps. So if we truncate away old information before we update the mark where the good information starts, then we get the race. We don't have a user-space interface reading, say, the clog, but it doesn't seem implausible for that to exist at some point. How would this code have to be structured then? > What I fear is: what > happens if a concurrent checkpoint reads the values between the two > operations, and a crash occurs? I think that the checkpoint might save > the updated values, so after crash recovery the truncate would not be > executed, possibly leaving files around. Leaving files around might be > dangerous for multixacts at least (it's probably harmless for xids). Why is leaving files around dangerous? If this is a problem, why is it not a problem for commit timestamps? I don't understand why these different SLRU uses are different. Yeah, we could go ahead with this patch as is and it might be fine, but I feel like we don't understand the broader issue completely yet. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
Peter Eisentraut wrote: > On 12/29/16 4:28 AM, Craig Ringer wrote: > > On 29 December 2016 at 16:51, Craig Ringer wrote: > >> On 28 December 2016 at 22:16, Petr Jelinek > >> wrote: > >> > >>> About the patch, it looks good to me for master with the minor exception > >>> that: > + appendStringInfo(buf, "pageno %d, xid %u", > + trunc.pageno, trunc.oldestXid); > >>> > >>> This should probably say oldestXid instead of xid in the text description. > >> > >> Agreed. > > > > Slightly amended attached. > > I've looked over this. It looks correct to me in principle. Thanks, pushed. I added a comment in vacuum.c, as well as removing the memcpy()s of the xlog record, which were unnecessary now that there's an intermediate struct. WAL version bumped. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
Robert Haas wrote: > On Thu, Jan 19, 2017 at 10:06 AM, Alvaro Herrera > wrote: > > Peter Eisentraut wrote: > > > >> Also, I wonder whether we should not in vacuum.c change the order of the > >> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just > >> to keep everything consistent. > > > > I am wary of doing that. The current coding is well battle-tested by > > now, but doing things in the opposite order, not at all. Pending some > > testing to show that there is no problem with a change, I would leave > > things as they are. Probably said testing is too onerous for the > > benefit (which is just a little consistency). What I fear is: what > > happens if a concurrent checkpoint reads the values between the two > > operations, and a crash occurs? I think that the checkpoint might save > > the updated values, so after crash recovery the truncate would not be > > executed, possibly leaving files around. Leaving files around might be > > dangerous for multixacts at least (it's probably harmless for xids). > > Don't both SLRUs eventually wrap? If so, leaving file around seems > dangerous for either in equal measure. Well, multixact uses the whole 2^32 space as valid, while xid only uses half of it. I think you would have to crash on every checkpoint just between those two points for two billion xacts for there to be a problem for xids. Anyway this just reinforces my argument that we shouldn't move those calls -- moving only the commit_ts one is good. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On Thu, Jan 19, 2017 at 10:06 AM, Alvaro Herrera wrote: > Peter Eisentraut wrote: > >> Also, I wonder whether we should not in vacuum.c change the order of the >> calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just >> to keep everything consistent. > > I am wary of doing that. The current coding is well battle-tested by > now, but doing things in the opposite order, not at all. Pending some > testing to show that there is no problem with a change, I would leave > things as they are. Probably said testing is too onerous for the > benefit (which is just a little consistency). What I fear is: what > happens if a concurrent checkpoint reads the values between the two > operations, and a crash occurs? I think that the checkpoint might save > the updated values, so after crash recovery the truncate would not be > executed, possibly leaving files around. Leaving files around might be > dangerous for multixacts at least (it's probably harmless for xids). Don't both SLRUs eventually wrap? If so, leaving file around seems dangerous for either in equal measure. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
Peter Eisentraut wrote: > Also, I wonder whether we should not in vacuum.c change the order of the > calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just > to keep everything consistent. I am wary of doing that. The current coding is well battle-tested by now, but doing things in the opposite order, not at all. Pending some testing to show that there is no problem with a change, I would leave things as they are. Probably said testing is too onerous for the benefit (which is just a little consistency). What I fear is: what happens if a concurrent checkpoint reads the values between the two operations, and a crash occurs? I think that the checkpoint might save the updated values, so after crash recovery the truncate would not be executed, possibly leaving files around. Leaving files around might be dangerous for multixacts at least (it's probably harmless for xids). -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 12/29/16 4:28 AM, Craig Ringer wrote: > On 29 December 2016 at 16:51, Craig Ringer wrote: >> On 28 December 2016 at 22:16, Petr Jelinek >> wrote: >> >>> About the patch, it looks good to me for master with the minor exception >>> that: + appendStringInfo(buf, "pageno %d, xid %u", + trunc.pageno, trunc.oldestXid); >>> >>> This should probably say oldestXid instead of xid in the text description. >> >> Agreed. > > Slightly amended attached. I've looked over this. It looks correct to me in principle. The commit message does not actually explain how the race on the standby is fixed by the patch. Also, I wonder whether we should not in vacuum.c change the order of the calls of SetTransactionIdLimit() and SetMultiXactIdLimit() as well, just to keep everything consistent. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 29 December 2016 at 16:51, Craig Ringer wrote:
> On 28 December 2016 at 22:16, Petr Jelinek
> wrote:
>
>> About the patch, it looks good to me for master with the minor exception
>> that:
>>> + appendStringInfo(buf, "pageno %d, xid %u",
>>> + trunc.pageno, trunc.oldestXid);
>>
>> This should probably say oldestXid instead of xid in the text description.
>
> Agreed.
Slightly amended attached.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
From e1dd1a711d65425df71db63d06ce6b6b934f12a8 Mon Sep 17 00:00:00 2001
From: Craig Ringer
Date: Wed, 28 Dec 2016 21:23:59 +0800
Subject: [PATCH] Fix a minor race between commit_ts slru truncation and
lookups
vac_truncate_clog was truncating the commit timestamp SLRU before
it advanced the oldest xid limit for commit timestamp lookups. This
created a small race window where a concurrent pg_xact_commit_timestamp
calling TransactionIdGetCommitTsData(...) could check the oldest
xid after the SLRU page containing it is truncated away but before
the threshold is updated.
Fix this by advancing the lower bound before removing the relevant SLRU pages.
A larger race window also existed on a standby. The lower bound for commit
timetamp validity was only advanced on a standby when we replayed a checkpoint.
This could happen a long time after the commit timestamp SLRU truncation.
This race only affects XIDs that vac_truncate_clog has determined are no longer
referenced in the system, so the minimum datfrozenxid has advanced past them.
It's never going to be triggered by code that looks up commit timestamps of
rows it's just read during an in-progress transaction. Also, the worst outcome
should the race be triggered is an I/O error from the SLRU code (where the
commit ts lookup should more correctly return null). So this race is pretty
harmless.
The nearly identical clog code has the same race, but it doesn't matter there
since there's no way for users to pass arbitrary XIDs to look them up in clog.
---
src/backend/access/rmgrdesc/committsdesc.c | 8 +---
src/backend/access/transam/commit_ts.c | 23 +++
src/backend/commands/vacuum.c | 3 ++-
src/include/access/commit_ts.h | 8
4 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
index 527e5dc..34553d5 100644
--- a/src/backend/access/rmgrdesc/committsdesc.c
+++ b/src/backend/access/rmgrdesc/committsdesc.c
@@ -33,10 +33,12 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
}
else if (info == COMMIT_TS_TRUNCATE)
{
- int pageno;
+ xl_commit_ts_truncate trunc;
- memcpy(&pageno, rec, sizeof(int));
- appendStringInfo(buf, "%d", pageno);
+ memcpy(&trunc, rec, SizeOfCommitTsTruncate);
+
+ appendStringInfo(buf, "pageno %d, oldestXid %u",
+ trunc.pageno, trunc.oldestXid);
}
else if (info == COMMIT_TS_SETTS)
{
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index a5b270c..86ac1d6 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -113,7 +113,7 @@ static bool CommitTsPagePrecedes(int page1, int page2);
static void ActivateCommitTs(void);
static void DeactivateCommitTs(void);
static void WriteZeroPageXlogRec(int pageno);
-static void WriteTruncateXlogRec(int pageno);
+static void WriteTruncateXlogRec(int pageno, TransactionId oldestXid);
static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
TransactionId *subxids, TimestampTz timestamp,
RepOriginId nodeid);
@@ -824,7 +824,7 @@ TruncateCommitTs(TransactionId oldestXact)
return; /* nothing to remove */
/* Write XLOG record */
- WriteTruncateXlogRec(cutoffPage);
+ WriteTruncateXlogRec(cutoffPage, oldestXact);
/* Now we can remove the old CommitTs segment(s) */
SimpleLruTruncate(CommitTsCtl, cutoffPage);
@@ -910,10 +910,15 @@ WriteZeroPageXlogRec(int pageno)
* Write a TRUNCATE xlog record
*/
static void
-WriteTruncateXlogRec(int pageno)
+WriteTruncateXlogRec(int pageno, TransactionId oldestXid)
{
+ xl_commit_ts_truncate xlrec;
+
+ xlrec.pageno = pageno;
+ xlrec.oldestXid = oldestXid;
+
XLogBeginInsert();
- XLogRegisterData((char *) (&pageno), sizeof(int));
+ XLogRegisterData((char *) (&xlrec), SizeOfCommitTsTruncate);
(void) XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_TRUNCATE);
}
@@ -967,17 +972,19 @@ commit_ts_redo(XLogReaderState *record)
}
else if (info == COMMIT_TS_TRUNCATE)
{
- int pageno;
+ xl_commit_ts_truncate trunc;
- memcpy(&pageno, XLogRecGetData(record), sizeof(int));
+ memcpy(&trunc, XLogRecGetData(record), SizeOfCommitTsTruncate);
+
+ AdvanceOldestCommitTsXid(trunc.oldestXid);
/*
* During XLOG replay, latest_page_number isn't set up yet; insert a
* suitable value to bypass the sanity test in
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 28 December 2016 at 22:16, Petr Jelinek wrote: > About the patch, it looks good to me for master with the minor exception > that: >> + appendStringInfo(buf, "pageno %d, xid %u", >> + trunc.pageno, trunc.oldestXid); > > This should probably say oldestXid instead of xid in the text description. Agreed. > About back-patching, I wonder if standby could be modified to move > oldestXid based on pageno reverse calculation, but it's going to be > slightly ugly. Not worth it IMO. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups
On 28/12/16 15:01, Craig Ringer wrote: > Hi all > > There's a minor race between commit_ts SLRU truncation and concurrent > commit_ts lookups, where a lookup can check the lower valid bound xid > without knowing it's already been truncated away. This would result in > a SLRU lookup error. > > It's pretty low-harm since it's hard to trigger and the only problem > is an error being emitted when we should otherwise return null/zero. > Most notably you have to pass an xid that used to be within the > datrozenxid but due to a concurrent vacuum has just moved outside it. > This can't happen if you're passing the xmin of a tuple that still > exists so it only matters for callers passing arbitrary XIDs in via > pg_xact_commit_timestamp(...). > > The race window is bigger on standby because there we don't find out > about the advance of the lower commit ts bound until the next > checkpoint. But you still have to be looking at very old xids that > don't exist on the heap anymore. > > We might as well fix it in HEAD, but it's totally pointless to > back-patch, and the only part of the race that can be realistically > hit is on standby, where we can't backpatch a fix w/o changing the > xlog format. Nope. We could narrow the scope by limiting commit_ts > slru truncation to just before a checkpoint, but given how hard this > is to hit... I don't care. > > (This came up as part of the investigation I've been doing on the > txid_status thread, where Robert pointed out a similar problem that > can arise where txid_status races with clog truncation. I noticed the > issue with standby while looking into that.) > Hi, I remember thinking this might affect committs when I was reading that thread but didn't have time to investigate it yet myself. Thanks for doing the all the work yourself. About the patch, it looks good to me for master with the minor exception that: > + appendStringInfo(buf, "pageno %d, xid %u", > + trunc.pageno, trunc.oldestXid); This should probably say oldestXid instead of xid in the text description. About back-patching, I wonder if standby could be modified to move oldestXid based on pageno reverse calculation, but it's going to be slightly ugly. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
