agora inbox for [email protected]
help / color / mirror / Atom feedRe: "SELECT ... FROM DUAL" is not quite as silly as it appears
24+ messages / 10 participants
[nested] [flat]
* Re: "SELECT ... FROM DUAL" is not quite as silly as it appears
@ 2018-10-14 22:11 Thomas Munro <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Thomas Munro @ 2018-10-14 22:11 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Fri, Mar 16, 2018 at 4:27 AM Tom Lane <[email protected]> wrote:
> We've long made fun of Oracle(TM) for the fact that if you just want
> to evaluate some expressions, you have to write "select ... from dual"
> rather than just "select ...". But I've realized recently that there's
> a bit of method in that madness after all.
We can still make fun of that table. Apparently it had two rows, so
you could double rows by cross joining against it, but at some point
one of them went missing, leaving a strange name behind. Source:
https://en.wikipedia.org/wiki/DUAL_table#History
--
Thomas Munro
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-17 21:06 Nathan Bossart <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Nathan Bossart @ 2023-04-17 21:06 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers
On Thu, Dec 15, 2022 at 03:22:59PM +1300, Thomas Munro wrote:
> While studying Jeff's new crop of collation patches I noticed in
> passing that check_strxfrm_bug() must surely by now be unnecessary.
> The buffer overrun bugs were fixed a decade ago, and the relevant
> systems are way out of support. If you're worried that the bugs might
> come back, then the test is insufficient: modern versions of both OSes
> have strxfrm_l(), which we aren't checking. In any case, we also
> completely disable this stuff because of bugs and quality problems in
> every other known implementation, via TRUST_STRXFRM (or rather the
> lack of it). So I think it's time to remove that function; please see
> attached.
Seems reasonable to me.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-17 21:48 Tom Lane <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Tom Lane @ 2023-04-17 21:48 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Thomas Munro <[email protected]>; pgsql-hackers
Nathan Bossart <[email protected]> writes:
> On Thu, Dec 15, 2022 at 03:22:59PM +1300, Thomas Munro wrote:
>> While studying Jeff's new crop of collation patches I noticed in
>> passing that check_strxfrm_bug() must surely by now be unnecessary.
>> The buffer overrun bugs were fixed a decade ago, and the relevant
>> systems are way out of support. If you're worried that the bugs might
>> come back, then the test is insufficient: modern versions of both OSes
>> have strxfrm_l(), which we aren't checking. In any case, we also
>> completely disable this stuff because of bugs and quality problems in
>> every other known implementation, via TRUST_STRXFRM (or rather the
>> lack of it). So I think it's time to remove that function; please see
>> attached.
> Seems reasonable to me.
+1. I wonder if we should go further and get rid of TRUST_STRXFRM
and the not-so-trivial amount of code around it (pg_strxfrm_enabled
etc). Carrying that indefinitely in the probably-vain hope that
the libraries will become trustworthy seems rather pointless.
Besides, if such a miracle does occur, we can dig the code out
of our git history.
regards, tom lane
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-17 22:40 Peter Geoghegan <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Peter Geoghegan @ 2023-04-17 22:40 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers
On Mon, Apr 17, 2023 at 2:48 PM Tom Lane <[email protected]> wrote:
> +1. I wonder if we should go further and get rid of TRUST_STRXFRM
> and the not-so-trivial amount of code around it (pg_strxfrm_enabled
> etc). Carrying that indefinitely in the probably-vain hope that
> the libraries will become trustworthy seems rather pointless.
> Besides, if such a miracle does occur, we can dig the code out
> of our git history.
+1 for getting rid of TRUST_STRXFRM.
ICU-based collations (which aren't affected by TRUST_STRXFRM) are
becoming the de facto standard (possibly even the de jure standard).
So even if we thought that the situation with strxfrm() had improved,
we'd still have little motivation to do anything about it.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-17 23:52 Michael Paquier <[email protected]>
parent: Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 24+ messages in thread
From: Michael Paquier @ 2023-04-17 23:52 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Jonathan S. Katz <[email protected]>; Amit Kapila <[email protected]>
On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote:
> On Mon, Apr 17, 2023 at 2:48 PM Tom Lane <[email protected]> wrote:
>> +1. I wonder if we should go further and get rid of TRUST_STRXFRM
>> and the not-so-trivial amount of code around it (pg_strxfrm_enabled
>> etc). Carrying that indefinitely in the probably-vain hope that
>> the libraries will become trustworthy seems rather pointless.
>> Besides, if such a miracle does occur, we can dig the code out
>> of our git history.
>
> +1 for getting rid of TRUST_STRXFRM.
>
> ICU-based collations (which aren't affected by TRUST_STRXFRM) are
> becoming the de facto standard (possibly even the de jure standard).
> So even if we thought that the situation with strxfrm() had improved,
> we'd still have little motivation to do anything about it.
Makes sense to do some cleanup now as this is new in the tree.
Perhaps somebody from the RMT would like to comment?
FYI, Jeff has also posted patches to replace this CFLAGS with a GUC:
https://www.postgresql.org/message-id/[email protected]
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-19 01:19 Thomas Munro <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 2 replies; 24+ messages in thread
From: Thomas Munro @ 2023-04-19 01:19 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Jonathan S. Katz <[email protected]>; Amit Kapila <[email protected]>
On Tue, Apr 18, 2023 at 11:52 AM Michael Paquier <[email protected]> wrote:
> On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote:
> > +1 for getting rid of TRUST_STRXFRM.
+1
The situation is not improving fast, and requires hard work to follow
on each OS. Clearly, mainstreaming ICU is the way to go. libc
support will always have niche uses, to be compatible with other
software on the box, but trusting strxfrm doesn't seem to be on the
cards any time soon.
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-19 02:31 Jonathan S. Katz <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 1 reply; 24+ messages in thread
From: Jonathan S. Katz @ 2023-04-19 02:31 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Amit Kapila <[email protected]>
On 4/18/23 9:19 PM, Thomas Munro wrote:
> On Tue, Apr 18, 2023 at 11:52 AM Michael Paquier <[email protected]> wrote:
>> On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote:
>>> +1 for getting rid of TRUST_STRXFRM.
>
> +1
>
> The situation is not improving fast, and requires hard work to follow
> on each OS. Clearly, mainstreaming ICU is the way to go. libc
> support will always have niche uses, to be compatible with other
> software on the box, but trusting strxfrm doesn't seem to be on the
> cards any time soon.
[RMT hat, personal opinion on RMT]
To be clear, is the proposal to remove both "check_strxfrm_bug" and
"TRUST_STRXFRM"?
Given a bunch of folks who have expertise in this area of code all agree
with removing the above as part of the collation cleanups targeted for
v16, I'm inclined to agree. I don't really see the need for an explicit
RMT action, but based on the consensus this seems OK to add as an open item.
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-19 14:40 Joe Conway <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 1 reply; 24+ messages in thread
From: Joe Conway @ 2023-04-19 14:40 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Jonathan S. Katz <[email protected]>; Amit Kapila <[email protected]>
On 4/18/23 21:19, Thomas Munro wrote:
> On Tue, Apr 18, 2023 at 11:52 AM Michael Paquier <[email protected]> wrote:
>> On Mon, Apr 17, 2023 at 03:40:14PM -0700, Peter Geoghegan wrote:
>> > +1 for getting rid of TRUST_STRXFRM.
>
> +1
>
> The situation is not improving fast, and requires hard work to follow
> on each OS. Clearly, mainstreaming ICU is the way to go. libc
> support will always have niche uses, to be compatible with other
> software on the box, but trusting strxfrm doesn't seem to be on the
> cards any time soon.
I have wondered a few times, given the known issues with strxfrm, how is
the use in selfuncs.c still ok. Has anyone taken a hard look at that?
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-19 15:01 Tom Lane <[email protected]>
parent: Joe Conway <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Tom Lane @ 2023-04-19 15:01 UTC (permalink / raw)
To: Joe Conway <[email protected]>; +Cc: Thomas Munro <[email protected]>; Michael Paquier <[email protected]>; Peter Geoghegan <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Jonathan S. Katz <[email protected]>; Amit Kapila <[email protected]>
Joe Conway <[email protected]> writes:
> I have wondered a few times, given the known issues with strxfrm, how is
> the use in selfuncs.c still ok. Has anyone taken a hard look at that?
On the one hand, we only need approximately-correct results in that
code. On the other, the result is fed to convert_string_to_scalar(),
which has a rather naive idea that it's dealing with ASCII text.
I've seen at least some strxfrm output that isn't even vaguely
textual-looking.
regards, tom lane
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-20 01:34 Thomas Munro <[email protected]>
parent: Jonathan S. Katz <[email protected]>
0 siblings, 2 replies; 24+ messages in thread
From: Thomas Munro @ 2023-04-20 01:34 UTC (permalink / raw)
To: Jonathan S. Katz <[email protected]>; +Cc: Michael Paquier <[email protected]>; Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Amit Kapila <[email protected]>
On Wed, Apr 19, 2023 at 2:31 PM Jonathan S. Katz <[email protected]> wrote:
> To be clear, is the proposal to remove both "check_strxfrm_bug" and
> "TRUST_STRXFRM"?
>
> Given a bunch of folks who have expertise in this area of code all agree
> with removing the above as part of the collation cleanups targeted for
> v16, I'm inclined to agree. I don't really see the need for an explicit
> RMT action, but based on the consensus this seems OK to add as an open item.
Thanks all. I went ahead and removed check_strxfrm_bug().
I could write a patch to remove the libc strxfrm support, but since
Jeff recently wrote new code in 16 to abstract that stuff, he might
prefer to look at it?
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-20 15:10 Jeff Davis <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 0 replies; 24+ messages in thread
From: Jeff Davis @ 2023-04-20 15:10 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; Jonathan S. Katz <[email protected]>; +Cc: Michael Paquier <[email protected]>; Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Amit Kapila <[email protected]>
On Thu, 2023-04-20 at 13:34 +1200, Thomas Munro wrote:
> I could write a patch to remove the libc strxfrm support, but since
> Jeff recently wrote new code in 16 to abstract that stuff, he might
> prefer to look at it?
+1 to removing it.
As far as how it's removed, we could directly check:
if (!collate_c && !(locale && locale->provider == COLLPROVIDER_ICU))
abbreviate = false;
as it was before, or we could still try to hide it as a detail behind a
function. I don't have a strong opinion there, though I thought it
might be good for varlena.c to not know those internal details.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 24+ messages in thread
* Re: check_strxfrm_bug()
@ 2023-04-22 19:14 Jonathan S. Katz <[email protected]>
parent: Thomas Munro <[email protected]>
1 sibling, 0 replies; 24+ messages in thread
From: Jonathan S. Katz @ 2023-04-22 19:14 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: Michael Paquier <[email protected]>; Peter Geoghegan <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; pgsql-hackers; Alvaro Herrera <[email protected]>; Amit Kapila <[email protected]>; Jeff Davis <[email protected]>
On 4/19/23 9:34 PM, Thomas Munro wrote:
> On Wed, Apr 19, 2023 at 2:31 PM Jonathan S. Katz <[email protected]> wrote:
>> To be clear, is the proposal to remove both "check_strxfrm_bug" and
>> "TRUST_STRXFRM"?
>>
>> Given a bunch of folks who have expertise in this area of code all agree
>> with removing the above as part of the collation cleanups targeted for
>> v16, I'm inclined to agree. I don't really see the need for an explicit
>> RMT action, but based on the consensus this seems OK to add as an open item.
>
> Thanks all. I went ahead and removed check_strxfrm_bug().
Thanks! For housekeeping, I put this into "Open Items" and marked it as
resolved.
> I could write a patch to remove the libc strxfrm support, but since
> Jeff recently wrote new code in 16 to abstract that stuff, he might
> prefer to look at it?
I believe we'd be qualifying this as an open item too? If so, let's add it.
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f164b7957ed..bc0a23da61b 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -523,6 +523,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -621,8 +627,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 45c4ae22e6a..dffbbd3cd3e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -314,6 +314,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 31 ++++++++-------
src/include/access/heapam.h | 3 ++
3 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 9edf6bf72d7..87f99497865 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -524,6 +524,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use frz_conflict_horizon as our cutoff for conflicts when
+ * the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin.
+ */
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -622,19 +640,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- /*
- * We can use frz_conflict_horizon as our cutoff for conflicts when
- * the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin.
- */
- if (!(presult->all_visible_except_removable && presult->all_frozen))
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(presult->frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
presult->frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b2a4caeb33a..02e33f213e1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -312,6 +312,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0012-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e715fc29a83..bac461940de 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -509,6 +509,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -607,8 +613,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 69d97bb8ece..d14f36d9ce7 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -315,6 +315,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f164b7957ed..bc0a23da61b 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -523,6 +523,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -621,8 +627,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 45c4ae22e6a..dffbbd3cd3e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -314,6 +314,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e715fc29a83..bac461940de 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -509,6 +509,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -607,8 +613,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 69d97bb8ece..d14f36d9ce7 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -315,6 +315,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index e715fc29a83..bac461940de 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -509,6 +509,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -607,8 +613,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 69d97bb8ece..d14f36d9ce7 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -315,6 +315,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 8 +++-
src/include/access/heapam.h | 3 ++
3 files changed, 42 insertions(+), 27 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f164b7957ed..bc0a23da61b 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -523,6 +523,12 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, frozen, presult->nfrozen);
+ frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -621,8 +627,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- frz_conflict_horizon = heap_frz_conflict_horizon(presult, pagefrz);
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 45c4ae22e6a..dffbbd3cd3e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -314,6 +314,9 @@ extern TransactionId heap_frz_conflict_horizon(PruneFreezeResult *presult,
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0010-Inline-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 31 ++++++++-------
src/include/access/heapam.h | 3 ++
3 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 9edf6bf72d7..87f99497865 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -524,6 +524,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use frz_conflict_horizon as our cutoff for conflicts when
+ * the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin.
+ */
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -622,19 +640,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- /*
- * We can use frz_conflict_horizon as our cutoff for conflicts when
- * the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin.
- */
- if (!(presult->all_visible_except_removable && presult->all_frozen))
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(presult->frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
presult->frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b2a4caeb33a..02e33f213e1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -312,6 +312,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0012-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier
@ 2024-01-07 21:53 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-01-07 21:53 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
Also move up the calculation of the freeze snapshot conflict horizon.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 31 ++++++++-------
src/include/access/heapam.h | 3 ++
3 files changed, 54 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 7261c4988d7..16e3f2520a4 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6659,35 +6659,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6726,6 +6710,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 9edf6bf72d7..87f99497865 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -524,6 +524,24 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use frz_conflict_horizon as our cutoff for conflicts when
+ * the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin.
+ */
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -622,19 +640,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- /*
- * We can use frz_conflict_horizon as our cutoff for conflicts when
- * the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin.
- */
- if (!(presult->all_visible_except_removable && presult->all_frozen))
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(presult->frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
presult->frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index b2a4caeb33a..02e33f213e1 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -312,6 +312,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0012-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v7 10/16] Separate tuple pre freeze checks and invoke earlier
@ 2024-03-26 00:54 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-03-26 00:54 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 41 +++++++++++---------
src/include/access/heapam.h | 3 ++
3 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e38c710c192..be48098f7f3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6657,35 +6657,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6724,6 +6708,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d38de9b063d..fe463ad7146 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -245,6 +245,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
PruneState prstate;
HeapTupleData tup;
TransactionId visibility_cutoff_xid;
+ TransactionId frz_conflict_horizon;
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
@@ -297,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* all-visible, so the conflict horizon remains InvalidTransactionId.
*/
presult->vm_conflict_horizon = visibility_cutoff_xid = InvalidTransactionId;
+ frz_conflict_horizon = InvalidTransactionId;
/* For advancing relfrozenxid and relminmxid */
presult->new_relfrozenxid = InvalidTransactionId;
@@ -541,6 +543,27 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use the visibility_cutoff_xid as our cutoff for conflicts
+ * when the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin. This avoids false conflicts when
+ * hot_standby_feedback is in use.
+ */
+ if (all_visible_except_removable && presult->all_frozen)
+ frz_conflict_horizon = visibility_cutoff_xid;
+ else
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -612,24 +635,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- TransactionId frz_conflict_horizon = InvalidTransactionId;
-
- /*
- * We can use the visibility_cutoff_xid as our cutoff for conflicts
- * when the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin. This avoids false conflicts when
- * hot_standby_feedback is in use.
- */
- if (all_visible_except_removable && presult->all_frozen)
- frz_conflict_horizon = visibility_cutoff_xid;
- else
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 6f9c66a872b..dbf6323b5ff 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -340,6 +340,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0011-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v9 10/21] Separate tuple pre freeze checks and invoke earlier
@ 2024-03-26 00:54 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-03-26 00:54 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 41 +++++++++++---------
src/include/access/heapam.h | 3 ++
3 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index bb856690234..b3119de2aa6 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6762,35 +6762,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6829,6 +6813,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f0decff35dc..13db348b2c1 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -245,6 +245,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
PruneState prstate;
HeapTupleData tup;
TransactionId visibility_cutoff_xid;
+ TransactionId frz_conflict_horizon;
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
@@ -297,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* all-visible, so the conflict horizon remains InvalidTransactionId.
*/
presult->vm_conflict_horizon = visibility_cutoff_xid = InvalidTransactionId;
+ frz_conflict_horizon = InvalidTransactionId;
/* For advancing relfrozenxid and relminmxid */
presult->new_relfrozenxid = InvalidTransactionId;
@@ -541,6 +543,27 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use the visibility_cutoff_xid as our cutoff for conflicts
+ * when the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin. This avoids false conflicts when
+ * hot_standby_feedback is in use.
+ */
+ if (all_visible_except_removable && presult->all_frozen)
+ frz_conflict_horizon = visibility_cutoff_xid;
+ else
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -612,24 +635,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- TransactionId frz_conflict_horizon = InvalidTransactionId;
-
- /*
- * We can use the visibility_cutoff_xid as our cutoff for conflicts
- * when the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin. This avoids false conflicts when
- * hot_standby_feedback is in use.
- */
- if (all_visible_except_removable && presult->all_frozen)
- frz_conflict_horizon = visibility_cutoff_xid;
- else
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index de11c166575..cc3b3346bc4 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -342,6 +342,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0011-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
* [PATCH v7 10/16] Separate tuple pre freeze checks and invoke earlier
@ 2024-03-26 00:54 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 24+ messages in thread
From: Melanie Plageman @ 2024-03-26 00:54 UTC (permalink / raw)
When combining the prune and freeze records their critical sections will
have to be combined. heap_freeze_execute_prepared() does a set of pre
freeze validations before starting its critical section. Move these
validations into a helper function, heap_pre_freeze_checks(), and invoke
it in heap_page_prune() before the pruning critical section.
---
src/backend/access/heap/heapam.c | 58 ++++++++++++++++-------------
src/backend/access/heap/pruneheap.c | 41 +++++++++++---------
src/include/access/heapam.h | 3 ++
3 files changed, 59 insertions(+), 43 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e38c710c192..be48098f7f3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6657,35 +6657,19 @@ heap_execute_freeze_tuple(HeapTupleHeader tuple, HeapTupleFreeze *frz)
}
/*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us. Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby). We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
- */
+* Perform xmin/xmax XID status sanity checks before calling
+* heap_freeze_execute_prepared().
+*
+* heap_prepare_freeze_tuple doesn't perform these checks directly because
+* pg_xact lookups are relatively expensive. They shouldn't be repeated
+* by successive VACUUMs that each decide against freezing the same page.
+*/
void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
- TransactionId snapshotConflictHorizon,
- HeapTupleFreeze *tuples, int ntuples)
+heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples)
{
Page page = BufferGetPage(buffer);
- Assert(ntuples > 0);
-
- /*
- * Perform xmin/xmax XID status sanity checks before critical section.
- *
- * heap_prepare_freeze_tuple doesn't perform these checks directly because
- * pg_xact lookups are relatively expensive. They shouldn't be repeated
- * by successive VACUUMs that each decide against freezing the same page.
- */
for (int i = 0; i < ntuples; i++)
{
HeapTupleFreeze *frz = tuples + i;
@@ -6724,6 +6708,30 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
xmax)));
}
}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Executes freezing of one or more heap tuples on a page on behalf of caller.
+ * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
+ * Caller must set 'offset' in each plan for us. Note that we destructively
+ * sort caller's tuples array in-place, so caller had better be done with it.
+ *
+ * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
+ * later on without any risk of unsafe pg_xact lookups, even following a hard
+ * crash (or when querying from a standby). We represent freezing by setting
+ * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
+ * See section on buffer access rules in src/backend/storage/buffer/README.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+ TransactionId snapshotConflictHorizon,
+ HeapTupleFreeze *tuples, int ntuples)
+{
+ Page page = BufferGetPage(buffer);
+
+ Assert(ntuples > 0);
START_CRIT_SECTION();
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d38de9b063d..fe463ad7146 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -245,6 +245,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
PruneState prstate;
HeapTupleData tup;
TransactionId visibility_cutoff_xid;
+ TransactionId frz_conflict_horizon;
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
@@ -297,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* all-visible, so the conflict horizon remains InvalidTransactionId.
*/
presult->vm_conflict_horizon = visibility_cutoff_xid = InvalidTransactionId;
+ frz_conflict_horizon = InvalidTransactionId;
/* For advancing relfrozenxid and relminmxid */
presult->new_relfrozenxid = InvalidTransactionId;
@@ -541,6 +543,27 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
(pagefrz->freeze_required ||
(whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ if (do_freeze)
+ {
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+
+ /*
+ * We can use the visibility_cutoff_xid as our cutoff for conflicts
+ * when the whole page is eligible to become all-frozen in the VM once
+ * we're done with it. Otherwise we generate a conservative cutoff by
+ * stepping back from OldestXmin. This avoids false conflicts when
+ * hot_standby_feedback is in use.
+ */
+ if (all_visible_except_removable && presult->all_frozen)
+ frz_conflict_horizon = visibility_cutoff_xid;
+ else
+ {
+ /* Avoids false conflicts when hot_standby_feedback in use */
+ frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(frz_conflict_horizon);
+ }
+ }
+
/* Any error while applying the changes is critical */
START_CRIT_SECTION();
@@ -612,24 +635,6 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
if (do_freeze)
{
- TransactionId frz_conflict_horizon = InvalidTransactionId;
-
- /*
- * We can use the visibility_cutoff_xid as our cutoff for conflicts
- * when the whole page is eligible to become all-frozen in the VM once
- * we're done with it. Otherwise we generate a conservative cutoff by
- * stepping back from OldestXmin. This avoids false conflicts when
- * hot_standby_feedback is in use.
- */
- if (all_visible_except_removable && presult->all_frozen)
- frz_conflict_horizon = visibility_cutoff_xid;
- else
- {
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
- }
-
/* Execute all freeze plans for page as a single atomic action */
heap_freeze_execute_prepared(relation, buffer,
frz_conflict_horizon,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 6f9c66a872b..dbf6323b5ff 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -340,6 +340,9 @@ extern void heap_inplace_update(Relation relation, HeapTuple tuple);
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
HeapPageFreeze *pagefrz,
HeapTupleFreeze *frz, bool *totally_frozen);
+
+extern void heap_pre_freeze_checks(Buffer buffer,
+ HeapTupleFreeze *tuples, int ntuples);
extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
TransactionId snapshotConflictHorizon,
HeapTupleFreeze *tuples, int ntuples);
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0011-Remove-heap_freeze_execute_prepared.patch"
^ permalink raw reply [nested|flat] 24+ messages in thread
end of thread, other threads:[~2024-03-26 00:54 UTC | newest]
Thread overview: 24+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-10-14 22:11 Re: "SELECT ... FROM DUAL" is not quite as silly as it appears Thomas Munro <[email protected]>
2023-04-17 21:06 Re: check_strxfrm_bug() Nathan Bossart <[email protected]>
2023-04-17 21:48 ` Re: check_strxfrm_bug() Tom Lane <[email protected]>
2023-04-17 22:40 ` Re: check_strxfrm_bug() Peter Geoghegan <[email protected]>
2023-04-17 23:52 ` Re: check_strxfrm_bug() Michael Paquier <[email protected]>
2023-04-19 01:19 ` Re: check_strxfrm_bug() Thomas Munro <[email protected]>
2023-04-19 02:31 ` Re: check_strxfrm_bug() Jonathan S. Katz <[email protected]>
2023-04-20 01:34 ` Re: check_strxfrm_bug() Thomas Munro <[email protected]>
2023-04-20 15:10 ` Re: check_strxfrm_bug() Jeff Davis <[email protected]>
2023-04-22 19:14 ` Re: check_strxfrm_bug() Jonathan S. Katz <[email protected]>
2023-04-19 14:40 ` Re: check_strxfrm_bug() Joe Conway <[email protected]>
2023-04-19 15:01 ` Re: check_strxfrm_bug() Tom Lane <[email protected]>
2024-01-07 21:53 [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v3 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v2 09/17] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-01-07 21:53 [PATCH v4 11/19] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-03-26 00:54 [PATCH v9 10/21] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-03-26 00:54 [PATCH v7 10/16] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[email protected]>
2024-03-26 00:54 [PATCH v7 10/16] Separate tuple pre freeze checks and invoke earlier Melanie Plageman <[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