public inbox for [email protected]help / color / mirror / Atom feed
[PATCH] Fix minor race in commit_ts SLRU truncation vs lookups 35+ messages / 4 participants [nested] [flat]
* [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups @ 2016-12-28 14:01 Craig Ringer <[email protected]> 2016-12-28 14:16 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Petr Jelinek <[email protected]> 0 siblings, 1 reply; 35+ messages in thread From: Craig Ringer @ 2016-12-28 14:01 UTC (permalink / raw) To: pgsql-hackers 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.) -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Attachments: [text/x-patch] 0001-Fix-a-minor-race-between-commit_ts-slru-truncation-a.patch (5.9K, ../../CAMsr+YFhVtRQT1VAwC+WGbbxZZRzNou=N9Ed-FrCqkwQ8H8oJQ@mail.gmail.com/2-0001-Fix-a-minor-race-between-commit_ts-slru-truncation-a.patch) download | inline diff: From 361a92ecbe45226ed25ce87da6a8c7bd749cca4a Mon Sep 17 00:00:00 2001 From: Craig Ringer <[email protected]> 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..2b117e4 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, xid %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 SimpleLruTruncate. */ - CommitTsCtl->shared->latest_page_number = pageno; + CommitTsCtl->shared->latest_page_number = trunc.pageno; - SimpleLruTruncate(CommitTsCtl, pageno); + SimpleLruTruncate(CommitTsCtl, trunc.pageno); } else if (info == COMMIT_TS_SETTS) { diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index b1be2f7..4336eda 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1152,6 +1152,8 @@ vac_truncate_clog(TransactionId frozenXID, if (bogus) return; + AdvanceOldestCommitTsXid(frozenXID); + /* * Truncate CLOG, multixact and CommitTs to the oldest computed value. */ @@ -1167,7 +1169,6 @@ vac_truncate_clog(TransactionId frozenXID, */ SetTransactionIdLimit(frozenXID, oldestxid_datoid); SetMultiXactIdLimit(minMulti, minmulti_datoid); - AdvanceOldestCommitTsXid(frozenXID); } diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h index cf343ca..06991c6 100644 --- a/src/include/access/commit_ts.h +++ b/src/include/access/commit_ts.h @@ -61,6 +61,14 @@ typedef struct xl_commit_ts_set #define SizeOfCommitTsSet (offsetof(xl_commit_ts_set, mainxid) + \ sizeof(TransactionId)) +typedef struct xl_commit_ts_truncate +{ + int pageno; + TransactionId oldestXid; +} xl_commit_ts_truncate; + +#define SizeOfCommitTsTruncate (offsetof(xl_commit_ts_truncate, oldestXid) +\ + sizeof(TransactionId)) extern void commit_ts_redo(XLogReaderState *record); extern void commit_ts_desc(StringInfo buf, XLogReaderState *record); -- 2.5.5 ^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups 2016-12-28 14:01 [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> @ 2016-12-28 14:16 ` Petr Jelinek <[email protected]> 2016-12-29 08:51 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> 0 siblings, 1 reply; 35+ messages in thread From: Petr Jelinek @ 2016-12-28 14:16 UTC (permalink / raw) To: Craig Ringer <[email protected]>; pgsql-hackers 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 ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers ^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups 2016-12-28 14:01 [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> 2016-12-28 14:16 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Petr Jelinek <[email protected]> @ 2016-12-29 08:51 ` Craig Ringer <[email protected]> 2016-12-29 09:28 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> 0 siblings, 1 reply; 35+ messages in thread From: Craig Ringer @ 2016-12-29 08:51 UTC (permalink / raw) To: Petr Jelinek <[email protected]>; +Cc: pgsql-hackers On 28 December 2016 at 22:16, Petr Jelinek <[email protected]> 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 ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers ^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups 2016-12-28 14:01 [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> 2016-12-28 14:16 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Petr Jelinek <[email protected]> 2016-12-29 08:51 ` Re: [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> @ 2016-12-29 09:28 ` Craig Ringer <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Craig Ringer @ 2016-12-29 09:28 UTC (permalink / raw) To: Petr Jelinek <[email protected]>; +Cc: pgsql-hackers On 29 December 2016 at 16:51, Craig Ringer <[email protected]> wrote: > On 28 December 2016 at 22:16, Petr Jelinek <[email protected]> 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 -- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers Attachments: [text/x-patch] 0001-Fix-a-minor-race-between-commit_ts-slru-truncation-a.patch (5.9K, ../../CAMsr+YGh0dO2f3hLNXYoOq92uNPeCrrH8qC3a9S8L2UnS9RD8g@mail.gmail.com/2-0001-Fix-a-minor-race-between-commit_ts-slru-truncation-a.patch) download | inline diff: From e1dd1a711d65425df71db63d06ce6b6b934f12a8 Mon Sep 17 00:00:00 2001 From: Craig Ringer <[email protected]> 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 SimpleLruTruncate. */ - CommitTsCtl->shared->latest_page_number = pageno; + CommitTsCtl->shared->latest_page_number = trunc.pageno; - SimpleLruTruncate(CommitTsCtl, pageno); + SimpleLruTruncate(CommitTsCtl, trunc.pageno); } else if (info == COMMIT_TS_SETTS) { diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index b1be2f7..4336eda 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1152,6 +1152,8 @@ vac_truncate_clog(TransactionId frozenXID, if (bogus) return; + AdvanceOldestCommitTsXid(frozenXID); + /* * Truncate CLOG, multixact and CommitTs to the oldest computed value. */ @@ -1167,7 +1169,6 @@ vac_truncate_clog(TransactionId frozenXID, */ SetTransactionIdLimit(frozenXID, oldestxid_datoid); SetMultiXactIdLimit(minMulti, minmulti_datoid); - AdvanceOldestCommitTsXid(frozenXID); } diff --git a/src/include/access/commit_ts.h b/src/include/access/commit_ts.h index cf343ca..06991c6 100644 --- a/src/include/access/commit_ts.h +++ b/src/include/access/commit_ts.h @@ -61,6 +61,14 @@ typedef struct xl_commit_ts_set #define SizeOfCommitTsSet (offsetof(xl_commit_ts_set, mainxid) + \ sizeof(TransactionId)) +typedef struct xl_commit_ts_truncate +{ + int pageno; + TransactionId oldestXid; +} xl_commit_ts_truncate; + +#define SizeOfCommitTsTruncate (offsetof(xl_commit_ts_truncate, oldestXid) +\ + sizeof(TransactionId)) extern void commit_ts_redo(XLogReaderState *record); extern void commit_ts_desc(StringInfo buf, XLogReaderState *record); -- 2.5.5 ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v7 7/9] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 867da6eebf..9aa2941457 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1538,8 +1538,7 @@ lookup_hash_entry(AggState *aggstate, uint32 hash) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.17.0 --UfEAyuTBtIjiZzX6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0008-Add-explain-MACHINE.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v4 7/7] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index f432825..1faea5b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1530,8 +1530,7 @@ lookup_hash_entry(AggState *aggstate, uint32 hash) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.7.4 --jI8keyz6grp/JLjh-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v3 7/7] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 396446b..472b2b1 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1499,8 +1499,7 @@ lookup_hash_entry(AggState *aggstate) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.7.4 --SNIs70sCzqvszXB4-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v5 7/7] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index c338ace..e5cd87d 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1535,8 +1535,7 @@ lookup_hash_entry(AggState *aggstate, uint32 hash) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.7.4 --M0YLxmUXciMpOLPE-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v6 7/7] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 3d60f7a314..567d65bb6c 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1538,8 +1538,7 @@ lookup_hash_entry(AggState *aggstate, uint32 hash) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.17.0 --xHbokkKX1kTiQeDC-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/21] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 1d1bf958b6..d80adc519d 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2070,8 +2070,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --jI8keyz6grp/JLjh Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v9 8/8] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index c6d03521e4..5cdd92acaf 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2058,8 +2058,7 @@ lookup_hash_entry(AggState *aggstate, uint32 hash, bool *in_hash_table) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --wwtQuX191/I956S7-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH 02/18] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 601b6dab03..394b4e667b 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2064,8 +2064,7 @@ initialize_hash_entry(AggState *aggstate, TupleHashTable hashtable, } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. * -- 2.17.0 --lc9FT7cWel8HagAv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-pg_restore-must-be-specified-and-list.patch" ^ permalink raw reply [nested|flat] 35+ messages in thread
* [PATCH v2 7/7] Update comment obsolete since 69c3936a @ 2020-02-15 21:53 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Justin Pryzby @ 2020-02-15 21:53 UTC (permalink / raw) --- src/backend/executor/nodeAgg.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index 4008e27..ab97a35 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1499,8 +1499,7 @@ lookup_hash_entry(AggState *aggstate) } /* - * Look up hash entries for the current tuple in all hashed grouping sets, - * returning an array of pergroup pointers suitable for advance_aggregates. + * Look up hash entries for the current tuple in all hashed grouping sets. * * Be aware that lookup_hash_entry can reset the tmpcontext. */ -- 2.7.4 --DiL7RhKs8rK9YGuF-- ^ permalink raw reply [nested|flat] 35+ messages in thread
* Re: lazy_scan_heap() should release lock on buffer before vacuuming FSM @ 2023-11-14 01:26 Andres Freund <[email protected]> 0 siblings, 0 replies; 35+ messages in thread From: Andres Freund @ 2023-11-14 01:26 UTC (permalink / raw) To: Melanie Plageman <[email protected]>; +Cc: pgsql-hackers Hi, On 2023-11-13 17:13:32 -0500, Melanie Plageman wrote: > I noticed that in lazy_scan_heap(), when there are no indexes on the > table being vacuumed, we don't release the lock on the heap page buffer > before vacuuming the freespace map. Other call sites of > FreeSpaceMapVacuumRange() hold no such lock. It seems like a waste to > hold a lock we don't need. I think this undersells the situation a bit. We right now do FreeSpaceMapVacuumRange() for 8GB of data (VACUUM_FSM_EVERY_PAGES) in the main fork, while holding an exclusive page level lock. There's no guarantee (or even high likelihood) that those pages are currently in the page cache, given that we have processed up to 8GB of heap data since. 8GB of data is roughly 2MB of FSM with the default compilation options. Of course processing 2MB of FSM doesn't take that long, but still, it seems worse than just reading a page or two. > diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c > index 6985d299b2..8b729828ce 100644 > --- a/src/backend/access/heap/vacuumlazy.c > +++ b/src/backend/access/heap/vacuumlazy.c > @@ -1046,18 +1046,6 @@ lazy_scan_heap(LVRelState *vacrel) > /* Forget the LP_DEAD items that we just vacuumed */ > dead_items->num_items = 0; > > - /* > - * Periodically perform FSM vacuuming to make newly-freed > - * space visible on upper FSM pages. Note we have not yet > - * performed FSM processing for blkno. > - */ > - if (blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES) > - { > - FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum, > - blkno); > - next_fsm_block_to_vacuum = blkno; > - } > - > /* > * Now perform FSM processing for blkno, and move on to next > * page. > @@ -1071,6 +1059,18 @@ lazy_scan_heap(LVRelState *vacrel) > > UnlockReleaseBuffer(buf); > RecordPageWithFreeSpace(vacrel->rel, blkno, freespace); > + > + /* > + * Periodically perform FSM vacuuming to make newly-freed > + * space visible on upper FSM pages. > + */ > + if (blkno - next_fsm_block_to_vacuum >= VACUUM_FSM_EVERY_PAGES) > + { > + FreeSpaceMapVacuumRange(vacrel->rel, next_fsm_block_to_vacuum, > + blkno); > + next_fsm_block_to_vacuum = blkno; > + } > + > continue; > } Previously there was this comment about "not yet", hinting at that being important for FreeSpaceMapVacuumRange's API. I think as-is we now don't vacuum the actually freshly updated page contents. FreeSpaceMapVacuumRange()'s comment says: * As above, but assume that only heap pages between start and end-1 inclusive * have new free-space information, so update only the upper-level slots * covering that block range. end == InvalidBlockNumber is equivalent to * "all the rest of the relation". So FreeSpaceMapVacuumRange(..., blkno) will not actually process the "effects" of the RecordPageWithFreeSpace() above it - which seems confusing. Aside: I just tried to reach the path and noticed something odd: =# show autovacuum; ┌────────────┐ │ autovacuum │ ├────────────┤ │ off │ └────────────┘ (1 row) =# \dt+ copytest_0 List of relations ┌────────┬────────────┬───────┬────────┬─────────────┬───────────────┬─────────┬─────────────┐ │ Schema │ Name │ Type │ Owner │ Persistence │ Access method │ Size │ Description │ ├────────┼────────────┼───────┼────────┼─────────────┼───────────────┼─────────┼─────────────┤ │ public │ copytest_0 │ table │ andres │ permanent │ heap │ 1143 MB │ │ └────────┴────────────┴───────┴────────┴─────────────┴───────────────┴─────────┴─────────────┘ =# DELETE FROM copytest_0; =# VACUUM (VERBOSE) copytest_0; ... INFO: 00000: table "copytest_0": truncated 146264 to 110934 pages ... tuples missed: 5848 dead from 89 pages not removed due to cleanup lock contention ... A bit of debugging later I figured out that this is due to the background writer. If I SIGSTOP bgwriter, the whole relation is truncated... Greetings, Andres Freund ^ permalink raw reply [nested|flat] 35+ messages in thread
end of thread, other threads:[~2023-11-14 01:26 UTC | newest] Thread overview: 35+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2016-12-28 14:01 [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups Craig Ringer <[email protected]> 2016-12-28 14:16 ` Petr Jelinek <[email protected]> 2016-12-29 08:51 ` Craig Ringer <[email protected]> 2016-12-29 09:28 ` Craig Ringer <[email protected]> 2020-02-15 21:53 [PATCH v7 7/9] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v4 7/7] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v3 7/7] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v5 7/7] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v6 7/7] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/21] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v9 8/8] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH 02/18] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2020-02-15 21:53 [PATCH v2 7/7] Update comment obsolete since 69c3936a Justin Pryzby <[email protected]> 2023-11-14 01:26 Re: lazy_scan_heap() should release lock on buffer before vacuuming FSM Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox