public inbox for [email protected]
help / color / mirror / Atom feedRe: O(n) tasks cause lengthy startups and checkpoints
29+ messages / 5 participants
[nested] [flat]
* Re: O(n) tasks cause lengthy startups and checkpoints
@ 2022-02-17 22:58 Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-02-17 22:58 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Thu, Feb 17, 2022 at 02:28:29PM -0800, Andres Freund wrote:
> As far as I understand, the primary concern are logical decoding serialized
> snapshots, because a lot of them can accumulate if there e.g. is an old unused
> / far behind slot. It should be easy to reduce the number of those snapshots
> by e.g. eliding some redundant ones. Perhaps we could also make backends in
> logical decoding occasionally do a bit of cleanup themselves.
>
> I've not seen reports of the number of mapping files to be an real issue?
I routinely see all four of these tasks impacting customers, but I'd say
the most common one is the temporary file cleanup. Besides eliminating
some redundant files and having backends perform some cleanup, what do you
think about skipping the logical decoding cleanup during
end-of-recovery/shutdown checkpoints? This was something that Bharath
brought up a while back [0]. As I noted in that thread, startup and
shutdown could still take a while if checkpoints are regularly delayed due
to logical decoding cleanup, but that might still help avoid a bit of
downtime.
> The improvements around deleting temporary files and serialized snapshots
> afaict don't require a dedicated process - they're only relevant during
> startup. We could use the approach of renaming the directory out of the way as
> done in this patchset but perform the cleanup in the startup process after
> we're up.
Perhaps this is a good place to start. As I mentioned above, IME the
temporary file cleanup is the most common problem, so I think even getting
that one fixed would be a huge improvement.
[0] https://postgr.es/m/CALj2ACXkkSL8EBpR7m%3DMt%3DyRGBhevcCs3x4fsp3Bc-D13yyHOg%40mail.gmail.com
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-02-17 23:12 ` Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Andres Freund @ 2022-02-17 23:12 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
Hi,
On 2022-02-17 14:58:38 -0800, Nathan Bossart wrote:
> On Thu, Feb 17, 2022 at 02:28:29PM -0800, Andres Freund wrote:
> > As far as I understand, the primary concern are logical decoding serialized
> > snapshots, because a lot of them can accumulate if there e.g. is an old unused
> > / far behind slot. It should be easy to reduce the number of those snapshots
> > by e.g. eliding some redundant ones. Perhaps we could also make backends in
> > logical decoding occasionally do a bit of cleanup themselves.
> >
> > I've not seen reports of the number of mapping files to be an real issue?
>
> I routinely see all four of these tasks impacting customers, but I'd say
> the most common one is the temporary file cleanup.
I took temp file cleanup and StartupReorderBuffer() "out of consideration" for
custodian, because they're not needed during normal running.
> Besides eliminating some redundant files and having backends perform some
> cleanup, what do you think about skipping the logical decoding cleanup
> during end-of-recovery/shutdown checkpoints?
I strongly disagree with it. Then you might never get the cleanup done, but
keep on operating until you hit corruption issues.
> > The improvements around deleting temporary files and serialized snapshots
> > afaict don't require a dedicated process - they're only relevant during
> > startup. We could use the approach of renaming the directory out of the way as
> > done in this patchset but perform the cleanup in the startup process after
> > we're up.
>
> Perhaps this is a good place to start. As I mentioned above, IME the
> temporary file cleanup is the most common problem, so I think even getting
> that one fixed would be a huge improvement.
Cool.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
@ 2022-02-18 16:44 ` Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-02-18 16:44 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Thu, Feb 17, 2022 at 03:12:47PM -0800, Andres Freund wrote:
>> > The improvements around deleting temporary files and serialized snapshots
>> > afaict don't require a dedicated process - they're only relevant during
>> > startup. We could use the approach of renaming the directory out of the way as
>> > done in this patchset but perform the cleanup in the startup process after
>> > we're up.
>>
>> Perhaps this is a good place to start. As I mentioned above, IME the
>> temporary file cleanup is the most common problem, so I think even getting
>> that one fixed would be a huge improvement.
>
> Cool.
Hm. How should this work for standbys? I can think of the following
options:
1. Do temporary file cleanup in the postmaster (as it does today).
2. Pause after allowing connections to clean up temporary files.
3. Do small amounts of temporary file cleanup whenever there is an
opportunity during recovery.
4. Wait until recovery completes before cleaning up temporary files.
I'm not too thrilled about any of these options.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-02-18 20:51 ` Nathan Bossart <[email protected]>
2022-03-17 23:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
0 siblings, 2 replies; 29+ messages in thread
From: Nathan Bossart @ 2022-02-18 20:51 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
> On Thu, Feb 17, 2022 at 03:12:47PM -0800, Andres Freund wrote:
>>> > The improvements around deleting temporary files and serialized snapshots
>>> > afaict don't require a dedicated process - they're only relevant during
>>> > startup. We could use the approach of renaming the directory out of the way as
>>> > done in this patchset but perform the cleanup in the startup process after
>>> > we're up.
BTW I know you don't like the dedicated process approach, but one
improvement to that approach could be to shut down the custodian process
when it has nothing to do.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-03-17 23:07 ` Nathan Bossart <[email protected]>
1 sibling, 0 replies; 29+ messages in thread
From: Nathan Bossart @ 2022-03-17 23:07 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
It seems unlikely that anything discussed in this thread will be committed
for v15, so I've adjusted the commitfest entry to v16 and moved it to the
next commitfest.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-06-23 11:58 ` Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
1 sibling, 1 reply; 29+ messages in thread
From: Simon Riggs @ 2022-06-23 11:58 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; Robert Haas <[email protected]>; pgsql-hackers
On Fri, 18 Feb 2022 at 20:51, Nathan Bossart <[email protected]> wrote:
>
> > On Thu, Feb 17, 2022 at 03:12:47PM -0800, Andres Freund wrote:
> >>> > The improvements around deleting temporary files and serialized snapshots
> >>> > afaict don't require a dedicated process - they're only relevant during
> >>> > startup. We could use the approach of renaming the directory out of the way as
> >>> > done in this patchset but perform the cleanup in the startup process after
> >>> > we're up.
>
> BTW I know you don't like the dedicated process approach, but one
> improvement to that approach could be to shut down the custodian process
> when it has nothing to do.
Having a central cleanup process makes a lot of sense. There is a long
list of potential tasks for such a process. My understanding is that
autovacuum already has an interface for handling additional workload
types, which is how BRIN indexes are handled. Do we really need a new
process? If so, lets do this now.
Nathan's point that certain tasks are blocking fast startup is a good
one and higher availability is a critical end goal. The thought that
we should complete these tasks during checkpoint is a good one, but
checkpoints should NOT be delayed by long running tasks that are
secondary to availability.
Andres' point that it would be better to avoid long running tasks is
good, if that is possible. That can be done better over time. This
point does not block the higher level goal of better availability
asap, so I support Nathan's overall proposals.
--
Simon Riggs http://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
@ 2022-06-23 13:46 ` Robert Haas <[email protected]>
2022-06-23 17:13 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 2 replies; 29+ messages in thread
From: Robert Haas @ 2022-06-23 13:46 UTC (permalink / raw)
To: Simon Riggs <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, Jun 23, 2022 at 7:58 AM Simon Riggs
<[email protected]> wrote:
> Having a central cleanup process makes a lot of sense. There is a long
> list of potential tasks for such a process. My understanding is that
> autovacuum already has an interface for handling additional workload
> types, which is how BRIN indexes are handled. Do we really need a new
> process?
It seems to me that if there's a long list of possible tasks for such
a process, that's actually a trickier situation than if there were
only one or two, because it may happen that when task X is really
urgent, the process is already busy with task Y.
I don't think that piggybacking more stuff onto autovacuum is a very
good idea for this exact reason. We already know that autovacuum
workers can get so busy that they can't keep up with the need to
vacuum and analyze tables. If we give them more things to do, that
figures to make it worse, at least on busy systems.
I do agree that a general mechanism for getting cleanup tasks done in
the background could be a useful thing to have, but I feel like it's
hard to see exactly how to make it work well. We can't just allow it
to spin up a million new processes, but at the same time, if it can't
guarantee that time-critical tasks get performed relatively quickly,
it's pretty worthless.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
@ 2022-06-23 17:13 ` Simon Riggs <[email protected]>
1 sibling, 0 replies; 29+ messages in thread
From: Simon Riggs @ 2022-06-23 17:13 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, 23 Jun 2022 at 14:46, Robert Haas <[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 7:58 AM Simon Riggs
> <[email protected]> wrote:
> > Having a central cleanup process makes a lot of sense. There is a long
> > list of potential tasks for such a process. My understanding is that
> > autovacuum already has an interface for handling additional workload
> > types, which is how BRIN indexes are handled. Do we really need a new
> > process?
>
> It seems to me that if there's a long list of possible tasks for such
> a process, that's actually a trickier situation than if there were
> only one or two, because it may happen that when task X is really
> urgent, the process is already busy with task Y.
>
> I don't think that piggybacking more stuff onto autovacuum is a very
> good idea for this exact reason. We already know that autovacuum
> workers can get so busy that they can't keep up with the need to
> vacuum and analyze tables. If we give them more things to do, that
> figures to make it worse, at least on busy systems.
>
> I do agree that a general mechanism for getting cleanup tasks done in
> the background could be a useful thing to have, but I feel like it's
> hard to see exactly how to make it work well. We can't just allow it
> to spin up a million new processes, but at the same time, if it can't
> guarantee that time-critical tasks get performed relatively quickly,
> it's pretty worthless.
Most of the tasks mentioned aren't time critical.
I have no objection to a new auxiliary process to execute those tasks,
which can be spawned when needed.
--
Simon Riggs http://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
@ 2022-06-23 17:15 ` Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
1 sibling, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-06-23 17:15 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Simon Riggs <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, Jun 23, 2022 at 09:46:28AM -0400, Robert Haas wrote:
> I do agree that a general mechanism for getting cleanup tasks done in
> the background could be a useful thing to have, but I feel like it's
> hard to see exactly how to make it work well. We can't just allow it
> to spin up a million new processes, but at the same time, if it can't
> guarantee that time-critical tasks get performed relatively quickly,
> it's pretty worthless.
My intent with this new auxiliary process is to offload tasks that aren't
particularly time-critical. They are only time-critical in the sense that
1) you might eventually run out of space and 2) you might encounter
wraparound with the logical replication files. But AFAICT these same risks
exist today in the checkpointer approach, although maybe not to the same
extent. In any case, 2 seems solvable to me outside of this patch set.
I'm grateful for the discussion in this thread so far, but I'm not seeing a
clear path forward. I'm glad to see threads like the one to stop doing
end-of-recovery checkpoints [0], but I don't know if it will be possible to
solve all of these availability concerns in a piecemeal fashion. I remain
open to exploring other suggested approaches beyond creating a new
auxiliary process.
[0] https://postgr.es/m/CA%2BTgmobrM2jvkiccCS9NgFcdjNSgAvk1qcAPx5S6F%2BoJT3D2mQ%40mail.gmail.com
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-06-24 10:45 ` Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Simon Riggs @ 2022-06-24 10:45 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, 23 Jun 2022 at 18:15, Nathan Bossart <[email protected]> wrote:
> I'm grateful for the discussion in this thread so far, but I'm not seeing a
> clear path forward.
+1 to add the new auxiliary process.
--
Simon Riggs http://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
@ 2022-07-02 22:05 ` Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-07-02 22:05 UTC (permalink / raw)
To: Simon Riggs <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Fri, Jun 24, 2022 at 11:45:22AM +0100, Simon Riggs wrote:
> On Thu, 23 Jun 2022 at 18:15, Nathan Bossart <[email protected]> wrote:
>> I'm grateful for the discussion in this thread so far, but I'm not seeing a
>> clear path forward.
>
> +1 to add the new auxiliary process.
I went ahead and put together a new patch set for this in which I've
attempted to address most of the feedback from upthread. Notably, I've
abandoned 0007 and 0008, added a way for processes to request specific
tasks for the custodian, and removed all the checks for
ShutdownRequestPending.
I haven't addressed the existing transaction ID wraparound risk with the
logical replication files. My instinct is that this deserveѕ its own
thread, and it might need to be considered a prerequisite to this change
based on the prior discussion here.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-07-02 22:54 ` Andres Freund <[email protected]>
2022-07-03 17:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Andres Freund @ 2022-07-02 22:54 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Hi,
On 2022-07-02 15:05:54 -0700, Nathan Bossart wrote:
> + /* Obtain requested tasks */
> + SpinLockAcquire(&CustodianShmem->cust_lck);
> + flags = CustodianShmem->cust_flags;
> + CustodianShmem->cust_flags = 0;
> + SpinLockRelease(&CustodianShmem->cust_lck);
Just resetting the flags to 0 is problematic. Consider what happens if there's
two tasks and and the one processed first errors out. You'll loose information
about needing to run the second task.
> + /* TODO: offloaded tasks go here */
Seems we're going to need some sorting of which tasks are most "urgent" / need
to be processed next if we plan to make this into some generic facility.
> +/*
> + * RequestCustodian
> + * Called to request a custodian task.
> + */
> +void
> +RequestCustodian(int flags)
> +{
> + SpinLockAcquire(&CustodianShmem->cust_lck);
> + CustodianShmem->cust_flags |= flags;
> + SpinLockRelease(&CustodianShmem->cust_lck);
> +
> + if (ProcGlobal->custodianLatch)
> + SetLatch(ProcGlobal->custodianLatch);
> +}
With this representation we can't really implement waiting for a task or
such. And it doesn't seem like a great API for the caller to just specify a
mix of flags.
> + /* Calculate how long to sleep */
> + end_time = (pg_time_t) time(NULL);
> + elapsed_secs = end_time - start_time;
> + if (elapsed_secs >= CUSTODIAN_TIMEOUT_S)
> + continue; /* no sleep for us */
> + cur_timeout = CUSTODIAN_TIMEOUT_S - elapsed_secs;
> +
> + (void) WaitLatch(MyLatch,
> + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
> + cur_timeout * 1000L /* convert to ms */ ,
> + WAIT_EVENT_CUSTODIAN_MAIN);
> + }
I don't think we should have this thing wake up on a regular basis. We're
doing way too much of that already, and I don't think we should add
more. Either we need a list of times when tasks need to be processed and wake
up at that time, or just wake up if somebody requests a task.
> From 5e95666efa31d6c8aa351e430c37ead6e27acb72 Mon Sep 17 00:00:00 2001
> From: Nathan Bossart <[email protected]>
> Date: Sun, 5 Dec 2021 21:16:44 -0800
> Subject: [PATCH v6 3/6] Split pgsql_tmp cleanup into two stages.
>
> First, pgsql_tmp directories will be renamed to stage them for
> removal. Then, all files in pgsql_tmp are removed before removing
> the staged directories themselves. This change is being made in
> preparation for a follow-up change to offload most temporary file
> cleanup to the new custodian process.
> Note that temporary relation files cannot be cleaned up via the
> aforementioned strategy and will not be offloaded to the custodian.
> ---
> src/backend/postmaster/postmaster.c | 8 +-
> src/backend/storage/file/fd.c | 174 ++++++++++++++++++++++++----
> src/include/storage/fd.h | 2 +-
> 3 files changed, 160 insertions(+), 24 deletions(-)
>
> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
> index e67370012f..82aa0c6307 100644
> --- a/src/backend/postmaster/postmaster.c
> +++ b/src/backend/postmaster/postmaster.c
> @@ -1402,7 +1402,8 @@ PostmasterMain(int argc, char *argv[])
> * Remove old temporary files. At this point there can be no other
> * Postgres processes running in this directory, so this should be safe.
> */
> - RemovePgTempFiles();
> + RemovePgTempFiles(true, true);
> + RemovePgTempFiles(false, false);
This is imo hard to read and easy to get wrong. Make it multiple functions or
pass named flags in.
> + * StagePgTempDirForRemoval
> + *
> + * This function renames the given directory with a special prefix that
> + * RemoveStagedPgTempDirs() will know to look for. An integer is appended to
> + * the end of the new directory name in case previously staged pgsql_tmp
> + * directories have not yet been removed.
> + */
It doesn't seem great to need to iterate through a directory that contains
other files, potentially a significant number. How about having a
staged_for_removal/ directory, and then only scanning that?
> +static void
> +StagePgTempDirForRemoval(const char *tmp_dir)
> +{
> + DIR *dir;
> + char stage_path[MAXPGPATH * 2];
> + char parent_path[MAXPGPATH * 2];
> + struct stat statbuf;
> +
> + /*
> + * If tmp_dir doesn't exist, there is nothing to stage.
> + */
> + dir = AllocateDir(tmp_dir);
> + if (dir == NULL)
> + {
> + if (errno != ENOENT)
> + ereport(LOG,
> + (errcode_for_file_access(),
> + errmsg("could not open directory \"%s\": %m", tmp_dir)));
> + return;
> + }
> + FreeDir(dir);
> +
> + strlcpy(parent_path, tmp_dir, MAXPGPATH * 2);
> + get_parent_directory(parent_path);
> +
> + /*
> + * get_parent_directory() returns an empty string if the input argument is
> + * just a file name (see comments in path.c), so handle that as being the
> + * current directory.
> + */
> + if (strlen(parent_path) == 0)
> + strlcpy(parent_path, ".", MAXPGPATH * 2);
> +
> + /*
> + * Find a name for the stage directory. We just increment an integer at the
> + * end of the name until we find one that doesn't exist.
> + */
> + for (int n = 0; n <= INT_MAX; n++)
> + {
> + snprintf(stage_path, sizeof(stage_path), "%s/%s%d", parent_path,
> + PG_TEMP_DIR_TO_REMOVE_PREFIX, n);
> +
> + if (stat(stage_path, &statbuf) != 0)
> + {
> + if (errno == ENOENT)
> + break;
> +
> + ereport(LOG,
> + (errcode_for_file_access(),
> + errmsg("could not stat file \"%s\": %m", stage_path)));
> + return;
> + }
> +
> + stage_path[0] = '\0';
I still dislike this approach. Loops until INT_MAX, not interruptible... Can't
we prevent conflicts by adding a timestamp or such?
> + }
> +
> + /*
> + * In the unlikely event that we couldn't find a name for the stage
> + * directory, bail out.
> + */
> + if (stage_path[0] == '\0')
> + {
> + ereport(LOG,
> + (errmsg("could not stage \"%s\" for deletion",
> + tmp_dir)));
> + return;
> + }
That's imo very much not ok. Just continuing in unexpected situations is a
recipe for introducing bugs / being hard to debug.
> From 43042799b96b588a446c509637b5acf570e2a325 Mon Sep 17 00:00:00 2001
> From a58a6bb70785a557a150680b64cd8ce78ce1b73a Mon Sep 17 00:00:00 2001
> From: Nathan Bossart <[email protected]>
> Date: Sun, 5 Dec 2021 22:02:40 -0800
> Subject: [PATCH v6 5/6] Move removal of old serialized snapshots to custodian.
>
> This was only done during checkpoints because it was a convenient
> place to put it.
As mentioned before, having it done as part of checkpoints provides pretty
decent wraparound protection - yes, it's not theoretically perfect, but in
reality it's very unlikely you can have an xid wraparound within one
checkpoint. I've mentioned this before, so at the very least I'd like to see
this acknowledged in the commit message.
> However, if there are many snapshots to remove, it can significantly extend
> checkpoint time.
I'd really like to see a reproducer or profile for this...
> + /*
> + * Remove serialized snapshots that are no longer required by any
> + * logical replication slot.
> + *
> + * It is not important for these to be removed in single-user mode, so
> + * we don't need any extra handling outside of the custodian process for
> + * this.
> + */
I don't think this claim is correct.
> From 0add8bb19a4ee83c6a6ec1f313329d737bf304a5 Mon Sep 17 00:00:00 2001
> From: Nathan Bossart <[email protected]>
> Date: Sun, 12 Dec 2021 22:07:11 -0800
> Subject: [PATCH v6 6/6] Move removal of old logical rewrite mapping files to
> custodian.
>
> If there are many such files to remove, checkpoints can take much
> longer. To avoid this, move this work to the newly-introduced
> custodian process.
As above I'd like to know why this could take that long. What are you doing
that there's so many mapping files (which only exist for catalog tables!) that
this is a significant fraction of a checkpoint?
> ---
> src/backend/access/heap/rewriteheap.c | 79 +++++++++++++++++++++++----
> src/backend/postmaster/custodian.c | 44 +++++++++++++++
> src/include/access/rewriteheap.h | 1 +
> src/include/postmaster/custodian.h | 5 ++
> 4 files changed, 119 insertions(+), 10 deletions(-)
>
> diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
> index 2a53826736..edeab65e60 100644
> --- a/src/backend/access/heap/rewriteheap.c
> +++ b/src/backend/access/heap/rewriteheap.c
> @@ -116,6 +116,7 @@
> #include "lib/ilist.h"
> #include "miscadmin.h"
> #include "pgstat.h"
> +#include "postmaster/custodian.h"
> #include "replication/logical.h"
> #include "replication/slot.h"
> #include "storage/bufmgr.h"
> @@ -1182,7 +1183,8 @@ heap_xlog_logical_rewrite(XLogReaderState *r)
> * Perform a checkpoint for logical rewrite mappings
> *
> * This serves two tasks:
> - * 1) Remove all mappings not needed anymore based on the logical restart LSN
> + * 1) Alert the custodian to remove all mappings not needed anymore based on the
> + * logical restart LSN
> * 2) Flush all remaining mappings to disk, so that replay after a checkpoint
> * only has to deal with the parts of a mapping that have been written out
> * after the checkpoint started.
> @@ -1210,6 +1212,10 @@ CheckPointLogicalRewriteHeap(void)
> if (cutoff != InvalidXLogRecPtr && redo < cutoff)
> cutoff = redo;
>
> + /* let the custodian know what it can remove */
> + CustodianSetLogicalRewriteCutoff(cutoff);
Setting this variable in a custodian datastructure and then fetching it from
there seems architecturally wrong to me.
> + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS);
What about single user mode?
ISTM that RequestCustodian() needs to either assert out if called in single
user mode, or execute tasks immediately in that context.
> +
> +/*
> + * Remove all mappings not needed anymore based on the logical restart LSN saved
> + * by the checkpointer. We use this saved value instead of calling
> + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an
> + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to
> + * disk.
> + */
What interference could there be?
> +void
> +RemoveOldLogicalRewriteMappings(void)
> +{
> + XLogRecPtr cutoff;
> + DIR *mappings_dir;
> + struct dirent *mapping_de;
> + char path[MAXPGPATH + 20];
> + bool value_set = false;
> +
> + cutoff = CustodianGetLogicalRewriteCutoff(&value_set);
> + if (!value_set)
> + return;
Afaics nothing clears values_set - is that a good idea?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
@ 2022-07-03 17:07 ` Nathan Bossart <[email protected]>
2022-07-03 17:27 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-07-03 17:07 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Hi Andres,
Thanks for the prompt review.
On Sat, Jul 02, 2022 at 03:54:56PM -0700, Andres Freund wrote:
> On 2022-07-02 15:05:54 -0700, Nathan Bossart wrote:
>> + /* Obtain requested tasks */
>> + SpinLockAcquire(&CustodianShmem->cust_lck);
>> + flags = CustodianShmem->cust_flags;
>> + CustodianShmem->cust_flags = 0;
>> + SpinLockRelease(&CustodianShmem->cust_lck);
>
> Just resetting the flags to 0 is problematic. Consider what happens if there's
> two tasks and and the one processed first errors out. You'll loose information
> about needing to run the second task.
I think we also want to retry any failed tasks. The way v6 handles this is
by requesting all tasks after an exception. Another way to handle this
could be to reset each individual flag before the task is executed, and
then we could surround each one with a PG_CATCH block that resets the flag.
I'll do it this way in the next revision.
>> +/*
>> + * RequestCustodian
>> + * Called to request a custodian task.
>> + */
>> +void
>> +RequestCustodian(int flags)
>> +{
>> + SpinLockAcquire(&CustodianShmem->cust_lck);
>> + CustodianShmem->cust_flags |= flags;
>> + SpinLockRelease(&CustodianShmem->cust_lck);
>> +
>> + if (ProcGlobal->custodianLatch)
>> + SetLatch(ProcGlobal->custodianLatch);
>> +}
>
> With this representation we can't really implement waiting for a task or
> such. And it doesn't seem like a great API for the caller to just specify a
> mix of flags.
At the moment, the idea is that nothing should need to wait for a task
because the custodian only handles things that are relatively non-critical.
If that changes, this could probably be expanded to look more like
RequestCheckpoint().
What would you suggest using instead of a mix of flags?
>> + /* Calculate how long to sleep */
>> + end_time = (pg_time_t) time(NULL);
>> + elapsed_secs = end_time - start_time;
>> + if (elapsed_secs >= CUSTODIAN_TIMEOUT_S)
>> + continue; /* no sleep for us */
>> + cur_timeout = CUSTODIAN_TIMEOUT_S - elapsed_secs;
>> +
>> + (void) WaitLatch(MyLatch,
>> + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
>> + cur_timeout * 1000L /* convert to ms */ ,
>> + WAIT_EVENT_CUSTODIAN_MAIN);
>> + }
>
> I don't think we should have this thing wake up on a regular basis. We're
> doing way too much of that already, and I don't think we should add
> more. Either we need a list of times when tasks need to be processed and wake
> up at that time, or just wake up if somebody requests a task.
I agree. I will remove the timeout in the next revision.
>> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
>> index e67370012f..82aa0c6307 100644
>> --- a/src/backend/postmaster/postmaster.c
>> +++ b/src/backend/postmaster/postmaster.c
>> @@ -1402,7 +1402,8 @@ PostmasterMain(int argc, char *argv[])
>> * Remove old temporary files. At this point there can be no other
>> * Postgres processes running in this directory, so this should be safe.
>> */
>> - RemovePgTempFiles();
>> + RemovePgTempFiles(true, true);
>> + RemovePgTempFiles(false, false);
>
> This is imo hard to read and easy to get wrong. Make it multiple functions or
> pass named flags in.
Will do.
>> + * StagePgTempDirForRemoval
>> + *
>> + * This function renames the given directory with a special prefix that
>> + * RemoveStagedPgTempDirs() will know to look for. An integer is appended to
>> + * the end of the new directory name in case previously staged pgsql_tmp
>> + * directories have not yet been removed.
>> + */
>
> It doesn't seem great to need to iterate through a directory that contains
> other files, potentially a significant number. How about having a
> staged_for_removal/ directory, and then only scanning that?
Yeah, that seems like a good idea. Will do.
>> + /*
>> + * Find a name for the stage directory. We just increment an integer at the
>> + * end of the name until we find one that doesn't exist.
>> + */
>> + for (int n = 0; n <= INT_MAX; n++)
>> + {
>> + snprintf(stage_path, sizeof(stage_path), "%s/%s%d", parent_path,
>> + PG_TEMP_DIR_TO_REMOVE_PREFIX, n);
>> +
>> + if (stat(stage_path, &statbuf) != 0)
>> + {
>> + if (errno == ENOENT)
>> + break;
>> +
>> + ereport(LOG,
>> + (errcode_for_file_access(),
>> + errmsg("could not stat file \"%s\": %m", stage_path)));
>> + return;
>> + }
>> +
>> + stage_path[0] = '\0';
>
> I still dislike this approach. Loops until INT_MAX, not interruptible... Can't
> we prevent conflicts by adding a timestamp or such?
I suppose it's highly unlikely that we'd see a conflict if we used the
timestamp instead. I'll do it this way in the next revision if that seems
good enough.
>> From a58a6bb70785a557a150680b64cd8ce78ce1b73a Mon Sep 17 00:00:00 2001
>> From: Nathan Bossart <[email protected]>
>> Date: Sun, 5 Dec 2021 22:02:40 -0800
>> Subject: [PATCH v6 5/6] Move removal of old serialized snapshots to custodian.
>>
>> This was only done during checkpoints because it was a convenient
>> place to put it.
>
> As mentioned before, having it done as part of checkpoints provides pretty
> decent wraparound protection - yes, it's not theoretically perfect, but in
> reality it's very unlikely you can have an xid wraparound within one
> checkpoint. I've mentioned this before, so at the very least I'd like to see
> this acknowledged in the commit message.
Will do.
>> + /* let the custodian know what it can remove */
>> + CustodianSetLogicalRewriteCutoff(cutoff);
>
> Setting this variable in a custodian datastructure and then fetching it from
> there seems architecturally wrong to me.
Where do you think it should go? I previously had it in the checkpointer's
shared memory, but you didn't like that the functions were declared in
bgwriter.h (along with the other checkpoint stuff). If the checkpointer
shared memory is the right place, should we create checkpointer.h and use
that instead?
>> + RequestCustodian(CUSTODIAN_REMOVE_REWRITE_MAPPINGS);
>
> What about single user mode?
>
>
> ISTM that RequestCustodian() needs to either assert out if called in single
> user mode, or execute tasks immediately in that context.
I like the idea of executing the tasks immediately since that's what
happens today in single-user mode. I will try doing it that way.
>> +/*
>> + * Remove all mappings not needed anymore based on the logical restart LSN saved
>> + * by the checkpointer. We use this saved value instead of calling
>> + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an
>> + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to
>> + * disk.
>> + */
>
> What interference could there be?
My concern is that the custodian could obtain a later cutoff than what the
checkpointer does, which might cause files to be concurrently unlinked and
fsync'd. If we always use the checkpointer's cutoff, that shouldn't be a
problem. This could probably be better explained in this comment.
>> +void
>> +RemoveOldLogicalRewriteMappings(void)
>> +{
>> + XLogRecPtr cutoff;
>> + DIR *mappings_dir;
>> + struct dirent *mapping_de;
>> + char path[MAXPGPATH + 20];
>> + bool value_set = false;
>> +
>> + cutoff = CustodianGetLogicalRewriteCutoff(&value_set);
>> + if (!value_set)
>> + return;
>
> Afaics nothing clears values_set - is that a good idea?
I'm using value_set to differentiate the case where InvalidXLogRecPtr means
the checkpointer hasn't determined a value yet versus the case where it
has. In the former, we don't want to take any action. In the latter, we
want to unlink all the files. Since we're moving to a request model for
the custodian, I might be able to remove this value_set stuff completely.
If that's not possible, it probably deserves a better comment.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-03 17:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-07-03 17:27 ` Andres Freund <[email protected]>
2022-07-06 16:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Andres Freund @ 2022-07-03 17:27 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Hi,
On 2022-07-03 10:07:54 -0700, Nathan Bossart wrote:
> Thanks for the prompt review.
>
> On Sat, Jul 02, 2022 at 03:54:56PM -0700, Andres Freund wrote:
> > On 2022-07-02 15:05:54 -0700, Nathan Bossart wrote:
> >> + /* Obtain requested tasks */
> >> + SpinLockAcquire(&CustodianShmem->cust_lck);
> >> + flags = CustodianShmem->cust_flags;
> >> + CustodianShmem->cust_flags = 0;
> >> + SpinLockRelease(&CustodianShmem->cust_lck);
> >
> > Just resetting the flags to 0 is problematic. Consider what happens if there's
> > two tasks and and the one processed first errors out. You'll loose information
> > about needing to run the second task.
>
> I think we also want to retry any failed tasks.
I don't think so, at least not if it's just going to retry that task straight
away - then we'll get stuck on that one task forever. If we had the ability to
"queue" it the end, to be processed after other already dequeued tasks, it'd
be a different story.
> The way v6 handles this is by requesting all tasks after an exception.
Ick. That strikes me as a bad idea.
> >> +/*
> >> + * RequestCustodian
> >> + * Called to request a custodian task.
> >> + */
> >> +void
> >> +RequestCustodian(int flags)
> >> +{
> >> + SpinLockAcquire(&CustodianShmem->cust_lck);
> >> + CustodianShmem->cust_flags |= flags;
> >> + SpinLockRelease(&CustodianShmem->cust_lck);
> >> +
> >> + if (ProcGlobal->custodianLatch)
> >> + SetLatch(ProcGlobal->custodianLatch);
> >> +}
> >
> > With this representation we can't really implement waiting for a task or
> > such. And it doesn't seem like a great API for the caller to just specify a
> > mix of flags.
>
> At the moment, the idea is that nothing should need to wait for a task
> because the custodian only handles things that are relatively non-critical.
Which is just plainly not true as the patchset stands...
I think we're going to have to block if some cleanup as part of a checkpoint
hasn't been completed by the next checkpoint - otherwise it'll just end up
being way too confusing and there's absolutely no backpressure anymore.
> If that changes, this could probably be expanded to look more like
> RequestCheckpoint().
>
> What would you suggest using instead of a mix of flags?
I suspect an array of tasks with requested and completed counters or such?
With a condition variable to wait on?
> >> + /* let the custodian know what it can remove */
> >> + CustodianSetLogicalRewriteCutoff(cutoff);
> >
> > Setting this variable in a custodian datastructure and then fetching it from
> > there seems architecturally wrong to me.
>
> Where do you think it should go? I previously had it in the checkpointer's
> shared memory, but you didn't like that the functions were declared in
> bgwriter.h (along with the other checkpoint stuff). If the checkpointer
> shared memory is the right place, should we create checkpointer.h and use
> that instead?
Well, so far I have not understood what the whole point of the shared state
is, so i have a bit of a hard time answering this ;)
> >> +/*
> >> + * Remove all mappings not needed anymore based on the logical restart LSN saved
> >> + * by the checkpointer. We use this saved value instead of calling
> >> + * ReplicationSlotsComputeLogicalRestartLSN() so that we don't interfere with an
> >> + * ongoing call to CheckPointLogicalRewriteHeap() that is flushing mappings to
> >> + * disk.
> >> + */
> >
> > What interference could there be?
>
> My concern is that the custodian could obtain a later cutoff than what the
> checkpointer does, which might cause files to be concurrently unlinked and
> fsync'd. If we always use the checkpointer's cutoff, that shouldn't be a
> problem. This could probably be better explained in this comment.
How about having a Datum argument to RequestCustodian() that is forwarded to
the task?
> >> +void
> >> +RemoveOldLogicalRewriteMappings(void)
> >> +{
> >> + XLogRecPtr cutoff;
> >> + DIR *mappings_dir;
> >> + struct dirent *mapping_de;
> >> + char path[MAXPGPATH + 20];
> >> + bool value_set = false;
> >> +
> >> + cutoff = CustodianGetLogicalRewriteCutoff(&value_set);
> >> + if (!value_set)
> >> + return;
> >
> > Afaics nothing clears values_set - is that a good idea?
>
> I'm using value_set to differentiate the case where InvalidXLogRecPtr means
> the checkpointer hasn't determined a value yet versus the case where it
> has. In the former, we don't want to take any action. In the latter, we
> want to unlink all the files. Since we're moving to a request model for
> the custodian, I might be able to remove this value_set stuff completely.
> If that's not possible, it probably deserves a better comment.
It would.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-03 17:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-03 17:27 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
@ 2022-07-06 16:51 ` Nathan Bossart <[email protected]>
2022-08-11 23:09 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-07-06 16:51 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
Here's a new revision where I've attempted to address all the feedback I've
received thus far. Notably, the custodian now uses a queue for registering
tasks and determining which tasks to execute. Other changes include
splitting the temporary file functions apart to avoid consecutive boolean
flags, using a timestamp instead of an integer for the staging name for
temporary directories, moving temporary directories to a dedicated
directory so that the custodian doesn't need to scan relation files,
ERROR-ing when something goes wrong when cleaning up temporary files,
executing requested tasks immediately in single-user mode, and more.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-03 17:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-03 17:27 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-06 16:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-08-11 23:09 ` Nathan Bossart <[email protected]>
2022-08-24 16:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Nathan Bossart @ 2022-08-11 23:09 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Wed, Jul 06, 2022 at 09:51:10AM -0700, Nathan Bossart wrote:
> Here's a new revision where I've attempted to address all the feedback I've
> received thus far. Notably, the custodian now uses a queue for registering
> tasks and determining which tasks to execute. Other changes include
> splitting the temporary file functions apart to avoid consecutive boolean
> flags, using a timestamp instead of an integer for the staging name for
> temporary directories, moving temporary directories to a dedicated
> directory so that the custodian doesn't need to scan relation files,
> ERROR-ing when something goes wrong when cleaning up temporary files,
> executing requested tasks immediately in single-user mode, and more.
Here is a rebased patch set for cfbot. There are no other differences
between v7 and v8.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: O(n) tasks cause lengthy startups and checkpoints
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-02-18 16:44 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-06-23 13:46 ` Re: O(n) tasks cause lengthy startups and checkpoints Robert Haas <[email protected]>
2022-06-23 17:15 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Re: O(n) tasks cause lengthy startups and checkpoints Simon Riggs <[email protected]>
2022-07-02 22:05 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-03 17:07 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-07-03 17:27 ` Re: O(n) tasks cause lengthy startups and checkpoints Andres Freund <[email protected]>
2022-07-06 16:51 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-08-11 23:09 ` Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
@ 2022-08-24 16:46 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Nathan Bossart @ 2022-08-24 16:46 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Simon Riggs <[email protected]>; Robert Haas <[email protected]>; Bossart, Nathan <[email protected]>; Bharath Rupireddy <[email protected]>; Maxim Orlov <[email protected]>; Amul Sul <[email protected]>; Bruce Momjian <[email protected]>; pgsql-hackers
On Thu, Aug 11, 2022 at 04:09:21PM -0700, Nathan Bossart wrote:
> Here is a rebased patch set for cfbot. There are no other differences
> between v7 and v8.
Another rebase for cfbot.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v2 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca64c45d8a3..70d35a21e98 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -605,6 +605,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -615,10 +618,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -630,6 +660,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -643,56 +677,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v4 13/19] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
Eliminate xl_heap_freeze and XLOG_HEAP2_FREEZE record. When vacuum
freezes tuples, the information needed to replay those changes is
now recorded in the xl_heap_prune record.
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
The XLOG_HEAP2_PRUNE record is now bigger than it was previously and
bigger than the XLOG_HEAP2_FREEZE record. A future commit will
streamline the record.
---
src/backend/access/heap/heapam.c | 146 ++++------
src/backend/access/heap/pruneheap.c | 326 ++++++++++++-----------
src/backend/access/rmgrdesc/heapdesc.c | 95 ++++---
src/backend/replication/logical/decode.c | 1 -
src/include/access/heapam_xlog.h | 97 ++++---
5 files changed, 318 insertions(+), 347 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e47b56e7856..532868039d5 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8706,8 +8706,6 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
/*
* Handles XLOG_HEAP2_PRUNE record type.
- *
- * Acquires a full cleanup lock.
*/
static void
heap_xlog_prune(XLogReaderState *record)
@@ -8718,12 +8716,22 @@ heap_xlog_prune(XLogReaderState *record)
RelFileLocator rlocator;
BlockNumber blkno;
XLogRedoAction action;
+ bool get_cleanup_lock;
XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
/*
- * We're about to remove tuples. In Hot Standby mode, ensure that there's
- * no queries running for which the removed tuples are still visible.
+ * If there are dead, redirected, or unused items set unused by
+ * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
+ * PageRepairFragementation() which expects a full cleanup lock.
+ */
+ get_cleanup_lock = xlrec->nredirected > 0 ||
+ xlrec->ndead > 0 || xlrec->nunused > 0;
+
+ /*
+ * We are either about to remove tuples or freeze them. In Hot Standby
+ * mode, ensure that there's no queries running for which any removed
+ * tuples are still visible or which consider the frozen xids as running.
*/
if (InHotStandby)
ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
@@ -8731,38 +8739,69 @@ heap_xlog_prune(XLogReaderState *record)
rlocator);
/*
- * If we have a full-page image, restore it (using a cleanup lock) and
- * we're done.
+ * If we have a full-page image, restore it and we're done.
*/
- action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, true,
- &buffer);
+ action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
+ get_cleanup_lock, &buffer);
+
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
- heap_page_prune_execute(buffer,
- redirected, nredirected,
- nowdead, ndead,
- nowunused, nunused);
+ if (nredirected > 0 || ndead > 0 || nunused > 0)
+ heap_page_prune_execute(buffer,
+ redirected, nredirected,
+ nowdead, ndead,
+ nowunused, nunused);
+
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
/*
* Note: we don't worry about updating the page's prunability hints.
@@ -9001,74 +9040,6 @@ heap_xlog_visible(XLogReaderState *record)
UnlockReleaseBuffer(vmbuffer);
}
-/*
- * Replay XLOG_HEAP2_FREEZE_PAGE records
- */
-static void
-heap_xlog_freeze_page(XLogReaderState *record)
-{
- XLogRecPtr lsn = record->EndRecPtr;
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) XLogRecGetData(record);
- Buffer buffer;
-
- /*
- * In Hot Standby mode, ensure that there's no queries running which still
- * consider the frozen xids as running.
- */
- if (InHotStandby)
- {
- RelFileLocator rlocator;
-
- XLogRecGetBlockTag(record, 0, &rlocator, NULL, NULL);
- ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
- xlrec->isCatalogRel,
- rlocator);
- }
-
- if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO)
- {
- Page page = BufferGetPage(buffer);
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
- int curoff = 0;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- for (int p = 0; p < xlrec->nplans; p++)
- {
- HeapTupleFreeze frz;
-
- /*
- * Convert freeze plan representation from WAL record into
- * per-tuple format used by heap_execute_freeze_tuple
- */
- frz.xmax = plans[p].xmax;
- frz.t_infomask2 = plans[p].t_infomask2;
- frz.t_infomask = plans[p].t_infomask;
- frz.frzflags = plans[p].frzflags;
- frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
-
- for (int i = 0; i < plans[p].ntuples; i++)
- {
- OffsetNumber offset = offsets[curoff++];
- ItemId lp;
- HeapTupleHeader tuple;
-
- lp = PageGetItemId(page, offset);
- tuple = (HeapTupleHeader) PageGetItem(page, lp);
- heap_execute_freeze_tuple(tuple, &frz);
- }
- }
-
- PageSetLSN(page, lsn);
- MarkBufferDirty(buffer);
- }
- if (BufferIsValid(buffer))
- UnlockReleaseBuffer(buffer);
-}
-
/*
* Given an "infobits" field from an XLog record, set the correct bits in the
* given infomask and infomask2 for the tuple touched by the record.
@@ -9975,9 +9946,6 @@ heap2_redo(XLogReaderState *record)
case XLOG_HEAP2_VACUUM:
heap_xlog_vacuum(record);
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- heap_xlog_freeze_page(record);
- break;
case XLOG_HEAP2_VISIBLE:
heap_xlog_visible(record);
break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7bd479cfd4e..19b50931b90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -79,6 +79,9 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
+static void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult);
+
/*
* Optionally prune and repair fragmentation in the specified page.
@@ -247,9 +250,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
HeapTupleData tup;
bool do_freeze;
bool do_prune;
+ bool do_hint;
bool whole_page_freezable;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -445,10 +448,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -498,14 +500,18 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
- /* Is the whole page freezable? And is there something to freeze */
+ /* Is the whole page freezable? And is there something to freeze? */
whole_page_freezable = presult->all_visible_except_removable &&
presult->all_frozen;
@@ -520,43 +526,51 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ do_freeze = false;
+ if (pagefrz)
+ {
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
+ {
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
+ }
+ }
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
if (do_freeze)
- {
heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
+ {
/*
- * 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 we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- 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);
- }
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
}
- /* Any error while applying the changes is critical */
START_CRIT_SECTION();
- /* Have we found any prunable items? */
- if (do_prune)
+ if (do_hint)
{
- /*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
- */
- heap_page_prune_execute(buffer,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
-
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -564,163 +578,159 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
- MarkBufferDirty(buffer);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+ if (do_prune || do_freeze)
+ {
/*
- * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
+ * Apply the planned item changes, then repair page fragmentation, and
+ * update the page's hint bit about whether it has free line pointers.
*/
- if (RelationNeedsWAL(relation))
+ if (do_prune)
{
- xl_heap_prune xlrec;
- XLogRecPtr recptr;
-
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
- xlrec.nredirected = prstate.nredirected;
- xlrec.ndead = prstate.ndead;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
-
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+ heap_page_prune_execute(buffer,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+ if (do_freeze)
+ {
/*
- * The OffsetNumber arrays are not actually in the buffer, but we
- * pretend that they are. When XLogInsert stores the whole
- * buffer, the offset arrays need not be stored too.
+ * 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. This avoids false
+ * conflicts when hot_standby_feedback is in use.
*/
- if (prstate.nredirected > 0)
- XLogRegisterBufData(0, (char *) prstate.redirected,
- prstate.nredirected *
- sizeof(OffsetNumber) * 2);
-
- if (prstate.ndead > 0)
- XLogRegisterBufData(0, (char *) prstate.nowdead,
- prstate.ndead * sizeof(OffsetNumber));
-
- if (prstate.nunused > 0)
- XLogRegisterBufData(0, (char *) prstate.nowunused,
- prstate.nunused * sizeof(OffsetNumber));
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ MarkBufferDirty(buffer);
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
- }
- else
- {
/*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
+ * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
*/
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
+ if (RelationNeedsWAL(relation))
+ log_heap_prune_and_freeze(relation, buffer, &prstate, presult);
}
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
+ /*
+ * If we froze tuples on the page, the caller can advance relfrozenxid and
+ * relminmxid to the values in pagefrz->FreezePageRelfrozenXid and
+ * pagefrz->FreezePageRelminMxid. Otherwise, it is only safe to advance to
+ * the values in pagefrz->NoFreezePage[RelfrozenXid|RelminMxid]
+ */
+ if (pagefrz)
{
- START_CRIT_SECTION();
+ if (presult->nfrozen > 0)
+ {
+ presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
+ }
+ else
+ {
+ presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
+ }
+ }
+}
- Assert(presult->nfrozen > 0);
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+static void
+log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult)
+{
+ xl_heap_prune xlrec;
+ XLogRecPtr recptr;
- MarkBufferDirty(buffer);
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+ bool do_freeze = presult->nfrozen > 0;
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
+ xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+ xlrec.nredirected = prstate->nredirected;
+ xlrec.ndead = prstate->ndead;
+ xlrec.nunused = prstate->nunused;
+ xlrec.nplans = 0;
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(prstate.frozen, presult->nfrozen, plans, offsets);
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions on
+ * the standby older than the youngest xmax of the most recently removed
+ * tuple this record will prune will conflict. If this record will freeze
+ * tuples, any transactions on the standby with xids older than the
+ * youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate->snapshotConflictHorizon,
+ presult->frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate->snapshotConflictHorizon;
- xlrec.snapshotConflictHorizon = presult->frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
+ /*
+ * Prepare deduplicated representation for use in WAL record Destructively
+ * sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(prstate->frozen,
+ presult->nfrozen, plans, offsets);
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
- XLogRegisterBufData(0, (char *) offsets,
- presult->nfrozen * sizeof(OffsetNumber));
+ XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ /*
+ * The OffsetNumber arrays are not actually in the buffer, but we pretend
+ * that they are. When XLogInsert stores the whole buffer, the offset
+ * arrays need not be stored too.
+ */
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
- PageSetLSN(page, recptr);
- }
+ if (prstate->nredirected > 0)
+ XLogRegisterBufData(0, (char *) prstate->redirected,
+ prstate->nredirected *
+ sizeof(OffsetNumber) * 2);
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
+ if (prstate->ndead > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowdead,
+ prstate->ndead * sizeof(OffsetNumber));
- /* Caller won't update new_relfrozenxid and new_relminmxid */
- if (!pagefrz)
- return;
+ if (prstate->nunused > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowunused,
+ prstate->nunused * sizeof(OffsetNumber));
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) offsets,
+ presult->nfrozen * sizeof(OffsetNumber));
- /*
- * If we will freeze tuples on the page or, even if we don't freeze tuples
- * on the page, if we will set the page all-frozen in the visibility map,
- * we can advance relfrozenxid and relminmxid to the values in
- * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
- */
- if (presult->all_frozen || presult->nfrozen > 0)
- {
- presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
- }
- else
- {
- presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
- }
-}
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ PageSetLSN(BufferGetPage(buffer), recptr);
+}
/*
* Perform visibility checks for heap pruning.
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 36a3d83c8c2..9f0a0341d40 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -179,43 +179,67 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
{
xl_heap_prune *xlrec = (xl_heap_prune *) rec;
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nredirected: %u, ndead: %u, isCatalogRel: %c",
+ appendStringInfo(buf, "snapshotConflictHorizon: %u, isCatalogRel: %c",
xlrec->snapshotConflictHorizon,
- xlrec->nredirected,
- xlrec->ndead,
xlrec->isCatalogRel ? 'T' : 'F');
if (XLogRecHasBlockData(record, 0))
{
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
+ int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0,
- &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
- end = (OffsetNumber *) ((char *) redirected + datalen);
- nowdead = redirected + (nredirected * 2);
- nowunused = nowdead + xlrec->ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ ndead = xlrec->ndead;
+ nunused = xlrec->nunused;
- appendStringInfo(buf, ", nunused: %d", nunused);
-
- appendStringInfoString(buf, ", redirected:");
- array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
- nredirected, &redirect_elem_desc, NULL);
- appendStringInfoString(buf, ", dead:");
- array_desc(buf, nowdead, sizeof(OffsetNumber), xlrec->ndead,
- &offset_elem_desc, NULL);
- appendStringInfoString(buf, ", unused:");
- array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
- &offset_elem_desc, NULL);
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
+ nowdead = redirected + (nredirected * 2);
+ nowunused = nowdead + ndead;
+ frz_offsets = nowunused + nunused;
+
+ appendStringInfo(buf, ", nredirected: %u, ndead: %u, nunused: %u, nplans: %u,",
+ nredirected,
+ ndead,
+ nunused,
+ nplans);
+
+ if (nredirected > 0)
+ {
+ appendStringInfoString(buf, ", redirected:");
+ array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
+ nredirected, &redirect_elem_desc, NULL);
+ }
+
+ if (ndead > 0)
+ {
+ appendStringInfoString(buf, ", dead:");
+ array_desc(buf, nowdead, sizeof(OffsetNumber), ndead,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nunused > 0)
+ {
+ appendStringInfoString(buf, ", unused:");
+ array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nplans > 0)
+ {
+ appendStringInfoString(buf, ", plans:");
+ array_desc(buf, plans, sizeof(xl_heap_freeze_plan), nplans,
+ &plan_elem_desc, &frz_offsets);
+ }
}
}
else if (info == XLOG_HEAP2_VACUUM)
@@ -235,28 +259,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
&offset_elem_desc, NULL);
}
}
- else if (info == XLOG_HEAP2_FREEZE_PAGE)
- {
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) rec;
-
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nplans: %u, isCatalogRel: %c",
- xlrec->snapshotConflictHorizon, xlrec->nplans,
- xlrec->isCatalogRel ? 'T' : 'F');
-
- if (XLogRecHasBlockData(record, 0))
- {
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- appendStringInfoString(buf, ", plans:");
- array_desc(buf, plans, sizeof(xl_heap_freeze_plan), xlrec->nplans,
- &plan_elem_desc, &offsets);
- }
- }
else if (info == XLOG_HEAP2_VISIBLE)
{
xl_heap_visible *xlrec = (xl_heap_visible *) rec;
@@ -361,9 +363,6 @@ heap2_identify(uint8 info)
case XLOG_HEAP2_VACUUM:
id = "VACUUM";
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- id = "FREEZE_PAGE";
- break;
case XLOG_HEAP2_VISIBLE:
id = "VISIBLE";
break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index e5ab7b78b78..f77051572fd 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -445,7 +445,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
* Everything else here is just low level physical stuff we're not
* interested in.
*/
- case XLOG_HEAP2_FREEZE_PAGE:
case XLOG_HEAP2_PRUNE:
case XLOG_HEAP2_VACUUM:
case XLOG_HEAP2_VISIBLE:
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..fe4a8ff0620 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -53,11 +53,10 @@
#define XLOG_HEAP2_REWRITE 0x00
#define XLOG_HEAP2_PRUNE 0x10
#define XLOG_HEAP2_VACUUM 0x20
-#define XLOG_HEAP2_FREEZE_PAGE 0x30
-#define XLOG_HEAP2_VISIBLE 0x40
-#define XLOG_HEAP2_MULTI_INSERT 0x50
-#define XLOG_HEAP2_LOCK_UPDATED 0x60
-#define XLOG_HEAP2_NEW_CID 0x70
+#define XLOG_HEAP2_VISIBLE 0x30
+#define XLOG_HEAP2_MULTI_INSERT 0x40
+#define XLOG_HEAP2_LOCK_UPDATED 0x50
+#define XLOG_HEAP2_NEW_CID 0x60
/*
* xl_heap_insert/xl_heap_multi_insert flag values, 8 bits are available.
@@ -226,28 +225,65 @@ typedef struct xl_heap_update
#define SizeOfHeapUpdate (offsetof(xl_heap_update, new_offnum) + sizeof(OffsetNumber))
+/*
+ * This struct represents a 'freeze plan', which describes how to freeze a
+ * group of one or more heap tuples (appears in xl_heap_prune record)
+ */
+/* 0x01 was XLH_FREEZE_XMIN */
+#define XLH_FREEZE_XVAC 0x02
+#define XLH_INVALID_XVAC 0x04
+
+typedef struct xl_heap_freeze_plan
+{
+ TransactionId xmax;
+ uint16 t_infomask2;
+ uint16 t_infomask;
+ uint8 frzflags;
+
+ /* Length of individual page offset numbers array for this plan */
+ uint16 ntuples;
+} xl_heap_freeze_plan;
+
+/*
+ * As of Postgres 17, XLOG_HEAP2_PRUNE records replace
+ * XLOG_HEAP2_FREEZE_PAGE records.
+ */
+
/*
* This is what we need to know about page pruning (both during VACUUM and
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
- * Acquires a full cleanup lock.
+ * Acquires a full cleanup lock if heap_page_prune_execute() must be called
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*--------------------------------------------------------------------
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ *--------------------------------------------------------------------
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
@@ -315,47 +351,6 @@ typedef struct xl_heap_inplace
#define SizeOfHeapInplace (offsetof(xl_heap_inplace, offnum) + sizeof(OffsetNumber))
-/*
- * This struct represents a 'freeze plan', which describes how to freeze a
- * group of one or more heap tuples (appears in xl_heap_freeze_page record)
- */
-/* 0x01 was XLH_FREEZE_XMIN */
-#define XLH_FREEZE_XVAC 0x02
-#define XLH_INVALID_XVAC 0x04
-
-typedef struct xl_heap_freeze_plan
-{
- TransactionId xmax;
- uint16 t_infomask2;
- uint16 t_infomask;
- uint8 frzflags;
-
- /* Length of individual page offset numbers array for this plan */
- uint16 ntuples;
-} xl_heap_freeze_plan;
-
-/*
- * This is what we need to know about a block being frozen during vacuum
- *
- * Backup block 0's data contains an array of xl_heap_freeze_plan structs
- * (with nplans elements), followed by one or more page offset number arrays.
- * Each such page offset number array corresponds to a single freeze plan
- * (REDO routine freezes corresponding heap tuples using freeze plan).
- */
-typedef struct xl_heap_freeze_page
-{
- TransactionId snapshotConflictHorizon;
- uint16 nplans;
- bool isCatalogRel; /* to handle recovery conflict during logical
- * decoding on standby */
-
- /*
- * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
- */
-} xl_heap_freeze_page;
-
-#define SizeOfHeapFreezePage (offsetof(xl_heap_freeze_page, isCatalogRel) + sizeof(bool))
-
/*
* This is what we need to know about setting a visibility map bit
*
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0014-Vacuum-second-pass-emits-XLOG_HEAP2_PRUNE-record.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v3 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d77270ad0d6..994cf75c54e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -619,6 +619,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -629,10 +632,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -644,6 +674,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -657,56 +691,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v4 13/19] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
Eliminate xl_heap_freeze and XLOG_HEAP2_FREEZE record. When vacuum
freezes tuples, the information needed to replay those changes is
now recorded in the xl_heap_prune record.
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
The XLOG_HEAP2_PRUNE record is now bigger than it was previously and
bigger than the XLOG_HEAP2_FREEZE record. A future commit will
streamline the record.
---
src/backend/access/heap/heapam.c | 146 ++++------
src/backend/access/heap/pruneheap.c | 326 ++++++++++++-----------
src/backend/access/rmgrdesc/heapdesc.c | 95 ++++---
src/backend/replication/logical/decode.c | 1 -
src/include/access/heapam_xlog.h | 97 ++++---
5 files changed, 318 insertions(+), 347 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e47b56e7856..532868039d5 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8706,8 +8706,6 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
/*
* Handles XLOG_HEAP2_PRUNE record type.
- *
- * Acquires a full cleanup lock.
*/
static void
heap_xlog_prune(XLogReaderState *record)
@@ -8718,12 +8716,22 @@ heap_xlog_prune(XLogReaderState *record)
RelFileLocator rlocator;
BlockNumber blkno;
XLogRedoAction action;
+ bool get_cleanup_lock;
XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
/*
- * We're about to remove tuples. In Hot Standby mode, ensure that there's
- * no queries running for which the removed tuples are still visible.
+ * If there are dead, redirected, or unused items set unused by
+ * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
+ * PageRepairFragementation() which expects a full cleanup lock.
+ */
+ get_cleanup_lock = xlrec->nredirected > 0 ||
+ xlrec->ndead > 0 || xlrec->nunused > 0;
+
+ /*
+ * We are either about to remove tuples or freeze them. In Hot Standby
+ * mode, ensure that there's no queries running for which any removed
+ * tuples are still visible or which consider the frozen xids as running.
*/
if (InHotStandby)
ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
@@ -8731,38 +8739,69 @@ heap_xlog_prune(XLogReaderState *record)
rlocator);
/*
- * If we have a full-page image, restore it (using a cleanup lock) and
- * we're done.
+ * If we have a full-page image, restore it and we're done.
*/
- action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, true,
- &buffer);
+ action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
+ get_cleanup_lock, &buffer);
+
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
- heap_page_prune_execute(buffer,
- redirected, nredirected,
- nowdead, ndead,
- nowunused, nunused);
+ if (nredirected > 0 || ndead > 0 || nunused > 0)
+ heap_page_prune_execute(buffer,
+ redirected, nredirected,
+ nowdead, ndead,
+ nowunused, nunused);
+
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
/*
* Note: we don't worry about updating the page's prunability hints.
@@ -9001,74 +9040,6 @@ heap_xlog_visible(XLogReaderState *record)
UnlockReleaseBuffer(vmbuffer);
}
-/*
- * Replay XLOG_HEAP2_FREEZE_PAGE records
- */
-static void
-heap_xlog_freeze_page(XLogReaderState *record)
-{
- XLogRecPtr lsn = record->EndRecPtr;
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) XLogRecGetData(record);
- Buffer buffer;
-
- /*
- * In Hot Standby mode, ensure that there's no queries running which still
- * consider the frozen xids as running.
- */
- if (InHotStandby)
- {
- RelFileLocator rlocator;
-
- XLogRecGetBlockTag(record, 0, &rlocator, NULL, NULL);
- ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
- xlrec->isCatalogRel,
- rlocator);
- }
-
- if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO)
- {
- Page page = BufferGetPage(buffer);
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
- int curoff = 0;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- for (int p = 0; p < xlrec->nplans; p++)
- {
- HeapTupleFreeze frz;
-
- /*
- * Convert freeze plan representation from WAL record into
- * per-tuple format used by heap_execute_freeze_tuple
- */
- frz.xmax = plans[p].xmax;
- frz.t_infomask2 = plans[p].t_infomask2;
- frz.t_infomask = plans[p].t_infomask;
- frz.frzflags = plans[p].frzflags;
- frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
-
- for (int i = 0; i < plans[p].ntuples; i++)
- {
- OffsetNumber offset = offsets[curoff++];
- ItemId lp;
- HeapTupleHeader tuple;
-
- lp = PageGetItemId(page, offset);
- tuple = (HeapTupleHeader) PageGetItem(page, lp);
- heap_execute_freeze_tuple(tuple, &frz);
- }
- }
-
- PageSetLSN(page, lsn);
- MarkBufferDirty(buffer);
- }
- if (BufferIsValid(buffer))
- UnlockReleaseBuffer(buffer);
-}
-
/*
* Given an "infobits" field from an XLog record, set the correct bits in the
* given infomask and infomask2 for the tuple touched by the record.
@@ -9975,9 +9946,6 @@ heap2_redo(XLogReaderState *record)
case XLOG_HEAP2_VACUUM:
heap_xlog_vacuum(record);
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- heap_xlog_freeze_page(record);
- break;
case XLOG_HEAP2_VISIBLE:
heap_xlog_visible(record);
break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7bd479cfd4e..19b50931b90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -79,6 +79,9 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
+static void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult);
+
/*
* Optionally prune and repair fragmentation in the specified page.
@@ -247,9 +250,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
HeapTupleData tup;
bool do_freeze;
bool do_prune;
+ bool do_hint;
bool whole_page_freezable;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -445,10 +448,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -498,14 +500,18 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
- /* Is the whole page freezable? And is there something to freeze */
+ /* Is the whole page freezable? And is there something to freeze? */
whole_page_freezable = presult->all_visible_except_removable &&
presult->all_frozen;
@@ -520,43 +526,51 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ do_freeze = false;
+ if (pagefrz)
+ {
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
+ {
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
+ }
+ }
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
if (do_freeze)
- {
heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
+ {
/*
- * 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 we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- 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);
- }
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
}
- /* Any error while applying the changes is critical */
START_CRIT_SECTION();
- /* Have we found any prunable items? */
- if (do_prune)
+ if (do_hint)
{
- /*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
- */
- heap_page_prune_execute(buffer,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
-
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -564,163 +578,159 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
- MarkBufferDirty(buffer);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+ if (do_prune || do_freeze)
+ {
/*
- * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
+ * Apply the planned item changes, then repair page fragmentation, and
+ * update the page's hint bit about whether it has free line pointers.
*/
- if (RelationNeedsWAL(relation))
+ if (do_prune)
{
- xl_heap_prune xlrec;
- XLogRecPtr recptr;
-
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
- xlrec.nredirected = prstate.nredirected;
- xlrec.ndead = prstate.ndead;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
-
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+ heap_page_prune_execute(buffer,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+ if (do_freeze)
+ {
/*
- * The OffsetNumber arrays are not actually in the buffer, but we
- * pretend that they are. When XLogInsert stores the whole
- * buffer, the offset arrays need not be stored too.
+ * 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. This avoids false
+ * conflicts when hot_standby_feedback is in use.
*/
- if (prstate.nredirected > 0)
- XLogRegisterBufData(0, (char *) prstate.redirected,
- prstate.nredirected *
- sizeof(OffsetNumber) * 2);
-
- if (prstate.ndead > 0)
- XLogRegisterBufData(0, (char *) prstate.nowdead,
- prstate.ndead * sizeof(OffsetNumber));
-
- if (prstate.nunused > 0)
- XLogRegisterBufData(0, (char *) prstate.nowunused,
- prstate.nunused * sizeof(OffsetNumber));
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ MarkBufferDirty(buffer);
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
- }
- else
- {
/*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
+ * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
*/
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
+ if (RelationNeedsWAL(relation))
+ log_heap_prune_and_freeze(relation, buffer, &prstate, presult);
}
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
+ /*
+ * If we froze tuples on the page, the caller can advance relfrozenxid and
+ * relminmxid to the values in pagefrz->FreezePageRelfrozenXid and
+ * pagefrz->FreezePageRelminMxid. Otherwise, it is only safe to advance to
+ * the values in pagefrz->NoFreezePage[RelfrozenXid|RelminMxid]
+ */
+ if (pagefrz)
{
- START_CRIT_SECTION();
+ if (presult->nfrozen > 0)
+ {
+ presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
+ }
+ else
+ {
+ presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
+ }
+ }
+}
- Assert(presult->nfrozen > 0);
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+static void
+log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult)
+{
+ xl_heap_prune xlrec;
+ XLogRecPtr recptr;
- MarkBufferDirty(buffer);
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+ bool do_freeze = presult->nfrozen > 0;
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
+ xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+ xlrec.nredirected = prstate->nredirected;
+ xlrec.ndead = prstate->ndead;
+ xlrec.nunused = prstate->nunused;
+ xlrec.nplans = 0;
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(prstate.frozen, presult->nfrozen, plans, offsets);
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions on
+ * the standby older than the youngest xmax of the most recently removed
+ * tuple this record will prune will conflict. If this record will freeze
+ * tuples, any transactions on the standby with xids older than the
+ * youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate->snapshotConflictHorizon,
+ presult->frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate->snapshotConflictHorizon;
- xlrec.snapshotConflictHorizon = presult->frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
+ /*
+ * Prepare deduplicated representation for use in WAL record Destructively
+ * sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(prstate->frozen,
+ presult->nfrozen, plans, offsets);
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
- XLogRegisterBufData(0, (char *) offsets,
- presult->nfrozen * sizeof(OffsetNumber));
+ XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ /*
+ * The OffsetNumber arrays are not actually in the buffer, but we pretend
+ * that they are. When XLogInsert stores the whole buffer, the offset
+ * arrays need not be stored too.
+ */
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
- PageSetLSN(page, recptr);
- }
+ if (prstate->nredirected > 0)
+ XLogRegisterBufData(0, (char *) prstate->redirected,
+ prstate->nredirected *
+ sizeof(OffsetNumber) * 2);
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
+ if (prstate->ndead > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowdead,
+ prstate->ndead * sizeof(OffsetNumber));
- /* Caller won't update new_relfrozenxid and new_relminmxid */
- if (!pagefrz)
- return;
+ if (prstate->nunused > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowunused,
+ prstate->nunused * sizeof(OffsetNumber));
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) offsets,
+ presult->nfrozen * sizeof(OffsetNumber));
- /*
- * If we will freeze tuples on the page or, even if we don't freeze tuples
- * on the page, if we will set the page all-frozen in the visibility map,
- * we can advance relfrozenxid and relminmxid to the values in
- * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
- */
- if (presult->all_frozen || presult->nfrozen > 0)
- {
- presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
- }
- else
- {
- presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
- }
-}
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ PageSetLSN(BufferGetPage(buffer), recptr);
+}
/*
* Perform visibility checks for heap pruning.
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 36a3d83c8c2..9f0a0341d40 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -179,43 +179,67 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
{
xl_heap_prune *xlrec = (xl_heap_prune *) rec;
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nredirected: %u, ndead: %u, isCatalogRel: %c",
+ appendStringInfo(buf, "snapshotConflictHorizon: %u, isCatalogRel: %c",
xlrec->snapshotConflictHorizon,
- xlrec->nredirected,
- xlrec->ndead,
xlrec->isCatalogRel ? 'T' : 'F');
if (XLogRecHasBlockData(record, 0))
{
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
+ int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0,
- &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
- end = (OffsetNumber *) ((char *) redirected + datalen);
- nowdead = redirected + (nredirected * 2);
- nowunused = nowdead + xlrec->ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ ndead = xlrec->ndead;
+ nunused = xlrec->nunused;
- appendStringInfo(buf, ", nunused: %d", nunused);
-
- appendStringInfoString(buf, ", redirected:");
- array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
- nredirected, &redirect_elem_desc, NULL);
- appendStringInfoString(buf, ", dead:");
- array_desc(buf, nowdead, sizeof(OffsetNumber), xlrec->ndead,
- &offset_elem_desc, NULL);
- appendStringInfoString(buf, ", unused:");
- array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
- &offset_elem_desc, NULL);
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
+ nowdead = redirected + (nredirected * 2);
+ nowunused = nowdead + ndead;
+ frz_offsets = nowunused + nunused;
+
+ appendStringInfo(buf, ", nredirected: %u, ndead: %u, nunused: %u, nplans: %u,",
+ nredirected,
+ ndead,
+ nunused,
+ nplans);
+
+ if (nredirected > 0)
+ {
+ appendStringInfoString(buf, ", redirected:");
+ array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
+ nredirected, &redirect_elem_desc, NULL);
+ }
+
+ if (ndead > 0)
+ {
+ appendStringInfoString(buf, ", dead:");
+ array_desc(buf, nowdead, sizeof(OffsetNumber), ndead,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nunused > 0)
+ {
+ appendStringInfoString(buf, ", unused:");
+ array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nplans > 0)
+ {
+ appendStringInfoString(buf, ", plans:");
+ array_desc(buf, plans, sizeof(xl_heap_freeze_plan), nplans,
+ &plan_elem_desc, &frz_offsets);
+ }
}
}
else if (info == XLOG_HEAP2_VACUUM)
@@ -235,28 +259,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
&offset_elem_desc, NULL);
}
}
- else if (info == XLOG_HEAP2_FREEZE_PAGE)
- {
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) rec;
-
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nplans: %u, isCatalogRel: %c",
- xlrec->snapshotConflictHorizon, xlrec->nplans,
- xlrec->isCatalogRel ? 'T' : 'F');
-
- if (XLogRecHasBlockData(record, 0))
- {
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- appendStringInfoString(buf, ", plans:");
- array_desc(buf, plans, sizeof(xl_heap_freeze_plan), xlrec->nplans,
- &plan_elem_desc, &offsets);
- }
- }
else if (info == XLOG_HEAP2_VISIBLE)
{
xl_heap_visible *xlrec = (xl_heap_visible *) rec;
@@ -361,9 +363,6 @@ heap2_identify(uint8 info)
case XLOG_HEAP2_VACUUM:
id = "VACUUM";
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- id = "FREEZE_PAGE";
- break;
case XLOG_HEAP2_VISIBLE:
id = "VISIBLE";
break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index e5ab7b78b78..f77051572fd 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -445,7 +445,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
* Everything else here is just low level physical stuff we're not
* interested in.
*/
- case XLOG_HEAP2_FREEZE_PAGE:
case XLOG_HEAP2_PRUNE:
case XLOG_HEAP2_VACUUM:
case XLOG_HEAP2_VISIBLE:
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..fe4a8ff0620 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -53,11 +53,10 @@
#define XLOG_HEAP2_REWRITE 0x00
#define XLOG_HEAP2_PRUNE 0x10
#define XLOG_HEAP2_VACUUM 0x20
-#define XLOG_HEAP2_FREEZE_PAGE 0x30
-#define XLOG_HEAP2_VISIBLE 0x40
-#define XLOG_HEAP2_MULTI_INSERT 0x50
-#define XLOG_HEAP2_LOCK_UPDATED 0x60
-#define XLOG_HEAP2_NEW_CID 0x70
+#define XLOG_HEAP2_VISIBLE 0x30
+#define XLOG_HEAP2_MULTI_INSERT 0x40
+#define XLOG_HEAP2_LOCK_UPDATED 0x50
+#define XLOG_HEAP2_NEW_CID 0x60
/*
* xl_heap_insert/xl_heap_multi_insert flag values, 8 bits are available.
@@ -226,28 +225,65 @@ typedef struct xl_heap_update
#define SizeOfHeapUpdate (offsetof(xl_heap_update, new_offnum) + sizeof(OffsetNumber))
+/*
+ * This struct represents a 'freeze plan', which describes how to freeze a
+ * group of one or more heap tuples (appears in xl_heap_prune record)
+ */
+/* 0x01 was XLH_FREEZE_XMIN */
+#define XLH_FREEZE_XVAC 0x02
+#define XLH_INVALID_XVAC 0x04
+
+typedef struct xl_heap_freeze_plan
+{
+ TransactionId xmax;
+ uint16 t_infomask2;
+ uint16 t_infomask;
+ uint8 frzflags;
+
+ /* Length of individual page offset numbers array for this plan */
+ uint16 ntuples;
+} xl_heap_freeze_plan;
+
+/*
+ * As of Postgres 17, XLOG_HEAP2_PRUNE records replace
+ * XLOG_HEAP2_FREEZE_PAGE records.
+ */
+
/*
* This is what we need to know about page pruning (both during VACUUM and
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
- * Acquires a full cleanup lock.
+ * Acquires a full cleanup lock if heap_page_prune_execute() must be called
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*--------------------------------------------------------------------
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ *--------------------------------------------------------------------
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
@@ -315,47 +351,6 @@ typedef struct xl_heap_inplace
#define SizeOfHeapInplace (offsetof(xl_heap_inplace, offnum) + sizeof(OffsetNumber))
-/*
- * This struct represents a 'freeze plan', which describes how to freeze a
- * group of one or more heap tuples (appears in xl_heap_freeze_page record)
- */
-/* 0x01 was XLH_FREEZE_XMIN */
-#define XLH_FREEZE_XVAC 0x02
-#define XLH_INVALID_XVAC 0x04
-
-typedef struct xl_heap_freeze_plan
-{
- TransactionId xmax;
- uint16 t_infomask2;
- uint16 t_infomask;
- uint8 frzflags;
-
- /* Length of individual page offset numbers array for this plan */
- uint16 ntuples;
-} xl_heap_freeze_plan;
-
-/*
- * This is what we need to know about a block being frozen during vacuum
- *
- * Backup block 0's data contains an array of xl_heap_freeze_plan structs
- * (with nplans elements), followed by one or more page offset number arrays.
- * Each such page offset number array corresponds to a single freeze plan
- * (REDO routine freezes corresponding heap tuples using freeze plan).
- */
-typedef struct xl_heap_freeze_page
-{
- TransactionId snapshotConflictHorizon;
- uint16 nplans;
- bool isCatalogRel; /* to handle recovery conflict during logical
- * decoding on standby */
-
- /*
- * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
- */
-} xl_heap_freeze_page;
-
-#define SizeOfHeapFreezePage (offsetof(xl_heap_freeze_page, isCatalogRel) + sizeof(bool))
-
/*
* This is what we need to know about setting a visibility map bit
*
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0014-Vacuum-second-pass-emits-XLOG_HEAP2_PRUNE-record.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v3 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d77270ad0d6..994cf75c54e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -619,6 +619,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -629,10 +632,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -644,6 +674,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -657,56 +691,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v4 13/19] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
Eliminate xl_heap_freeze and XLOG_HEAP2_FREEZE record. When vacuum
freezes tuples, the information needed to replay those changes is
now recorded in the xl_heap_prune record.
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
The XLOG_HEAP2_PRUNE record is now bigger than it was previously and
bigger than the XLOG_HEAP2_FREEZE record. A future commit will
streamline the record.
---
src/backend/access/heap/heapam.c | 146 ++++------
src/backend/access/heap/pruneheap.c | 326 ++++++++++++-----------
src/backend/access/rmgrdesc/heapdesc.c | 95 ++++---
src/backend/replication/logical/decode.c | 1 -
src/include/access/heapam_xlog.h | 97 ++++---
5 files changed, 318 insertions(+), 347 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index e47b56e7856..532868039d5 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8706,8 +8706,6 @@ ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
/*
* Handles XLOG_HEAP2_PRUNE record type.
- *
- * Acquires a full cleanup lock.
*/
static void
heap_xlog_prune(XLogReaderState *record)
@@ -8718,12 +8716,22 @@ heap_xlog_prune(XLogReaderState *record)
RelFileLocator rlocator;
BlockNumber blkno;
XLogRedoAction action;
+ bool get_cleanup_lock;
XLogRecGetBlockTag(record, 0, &rlocator, NULL, &blkno);
/*
- * We're about to remove tuples. In Hot Standby mode, ensure that there's
- * no queries running for which the removed tuples are still visible.
+ * If there are dead, redirected, or unused items set unused by
+ * heap_page_prune_and_freeze(), heap_page_prune_execute() will call
+ * PageRepairFragementation() which expects a full cleanup lock.
+ */
+ get_cleanup_lock = xlrec->nredirected > 0 ||
+ xlrec->ndead > 0 || xlrec->nunused > 0;
+
+ /*
+ * We are either about to remove tuples or freeze them. In Hot Standby
+ * mode, ensure that there's no queries running for which any removed
+ * tuples are still visible or which consider the frozen xids as running.
*/
if (InHotStandby)
ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
@@ -8731,38 +8739,69 @@ heap_xlog_prune(XLogReaderState *record)
rlocator);
/*
- * If we have a full-page image, restore it (using a cleanup lock) and
- * we're done.
+ * If we have a full-page image, restore it and we're done.
*/
- action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL, true,
- &buffer);
+ action = XLogReadBufferForRedoExtended(record, 0, RBM_NORMAL,
+ get_cleanup_lock, &buffer);
+
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
- heap_page_prune_execute(buffer,
- redirected, nredirected,
- nowdead, ndead,
- nowunused, nunused);
+ if (nredirected > 0 || ndead > 0 || nunused > 0)
+ heap_page_prune_execute(buffer,
+ redirected, nredirected,
+ nowdead, ndead,
+ nowunused, nunused);
+
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
/*
* Note: we don't worry about updating the page's prunability hints.
@@ -9001,74 +9040,6 @@ heap_xlog_visible(XLogReaderState *record)
UnlockReleaseBuffer(vmbuffer);
}
-/*
- * Replay XLOG_HEAP2_FREEZE_PAGE records
- */
-static void
-heap_xlog_freeze_page(XLogReaderState *record)
-{
- XLogRecPtr lsn = record->EndRecPtr;
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) XLogRecGetData(record);
- Buffer buffer;
-
- /*
- * In Hot Standby mode, ensure that there's no queries running which still
- * consider the frozen xids as running.
- */
- if (InHotStandby)
- {
- RelFileLocator rlocator;
-
- XLogRecGetBlockTag(record, 0, &rlocator, NULL, NULL);
- ResolveRecoveryConflictWithSnapshot(xlrec->snapshotConflictHorizon,
- xlrec->isCatalogRel,
- rlocator);
- }
-
- if (XLogReadBufferForRedo(record, 0, &buffer) == BLK_NEEDS_REDO)
- {
- Page page = BufferGetPage(buffer);
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
- int curoff = 0;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- for (int p = 0; p < xlrec->nplans; p++)
- {
- HeapTupleFreeze frz;
-
- /*
- * Convert freeze plan representation from WAL record into
- * per-tuple format used by heap_execute_freeze_tuple
- */
- frz.xmax = plans[p].xmax;
- frz.t_infomask2 = plans[p].t_infomask2;
- frz.t_infomask = plans[p].t_infomask;
- frz.frzflags = plans[p].frzflags;
- frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
-
- for (int i = 0; i < plans[p].ntuples; i++)
- {
- OffsetNumber offset = offsets[curoff++];
- ItemId lp;
- HeapTupleHeader tuple;
-
- lp = PageGetItemId(page, offset);
- tuple = (HeapTupleHeader) PageGetItem(page, lp);
- heap_execute_freeze_tuple(tuple, &frz);
- }
- }
-
- PageSetLSN(page, lsn);
- MarkBufferDirty(buffer);
- }
- if (BufferIsValid(buffer))
- UnlockReleaseBuffer(buffer);
-}
-
/*
* Given an "infobits" field from an XLog record, set the correct bits in the
* given infomask and infomask2 for the tuple touched by the record.
@@ -9975,9 +9946,6 @@ heap2_redo(XLogReaderState *record)
case XLOG_HEAP2_VACUUM:
heap_xlog_vacuum(record);
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- heap_xlog_freeze_page(record);
- break;
case XLOG_HEAP2_VISIBLE:
heap_xlog_visible(record);
break;
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 7bd479cfd4e..19b50931b90 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -79,6 +79,9 @@ static void heap_prune_record_dead_or_unused(PruneState *prstate, OffsetNumber o
static void heap_prune_record_unused(PruneState *prstate, OffsetNumber offnum);
static void page_verify_redirects(Page page);
+static void log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult);
+
/*
* Optionally prune and repair fragmentation in the specified page.
@@ -247,9 +250,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
HeapTupleData tup;
bool do_freeze;
bool do_prune;
+ bool do_hint;
bool whole_page_freezable;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -445,10 +448,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -498,14 +500,18 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
- /* Is the whole page freezable? And is there something to freeze */
+ /* Is the whole page freezable? And is there something to freeze? */
whole_page_freezable = presult->all_visible_except_removable &&
presult->all_frozen;
@@ -520,43 +526,51 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
+ do_freeze = false;
+ if (pagefrz)
+ {
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
+ {
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
+ }
+ }
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
if (do_freeze)
- {
heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
+ {
/*
- * 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 we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- 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);
- }
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
}
- /* Any error while applying the changes is critical */
START_CRIT_SECTION();
- /* Have we found any prunable items? */
- if (do_prune)
+ if (do_hint)
{
- /*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
- */
- heap_page_prune_execute(buffer,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
-
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -564,163 +578,159 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
- MarkBufferDirty(buffer);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+ if (do_prune || do_freeze)
+ {
/*
- * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
+ * Apply the planned item changes, then repair page fragmentation, and
+ * update the page's hint bit about whether it has free line pointers.
*/
- if (RelationNeedsWAL(relation))
+ if (do_prune)
{
- xl_heap_prune xlrec;
- XLogRecPtr recptr;
-
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
- xlrec.nredirected = prstate.nredirected;
- xlrec.ndead = prstate.ndead;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
-
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+ heap_page_prune_execute(buffer,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+ if (do_freeze)
+ {
/*
- * The OffsetNumber arrays are not actually in the buffer, but we
- * pretend that they are. When XLogInsert stores the whole
- * buffer, the offset arrays need not be stored too.
+ * 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. This avoids false
+ * conflicts when hot_standby_feedback is in use.
*/
- if (prstate.nredirected > 0)
- XLogRegisterBufData(0, (char *) prstate.redirected,
- prstate.nredirected *
- sizeof(OffsetNumber) * 2);
-
- if (prstate.ndead > 0)
- XLogRegisterBufData(0, (char *) prstate.nowdead,
- prstate.ndead * sizeof(OffsetNumber));
-
- if (prstate.nunused > 0)
- XLogRegisterBufData(0, (char *) prstate.nowunused,
- prstate.nunused * sizeof(OffsetNumber));
+ if (!(presult->all_visible_except_removable && presult->all_frozen))
+ {
+ presult->frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
+ TransactionIdRetreat(presult->frz_conflict_horizon);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ MarkBufferDirty(buffer);
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
- }
- else
- {
/*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
+ * Emit a WAL XLOG_HEAP2_PRUNE record showing what we did
*/
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
+ if (RelationNeedsWAL(relation))
+ log_heap_prune_and_freeze(relation, buffer, &prstate, presult);
}
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
+ /*
+ * If we froze tuples on the page, the caller can advance relfrozenxid and
+ * relminmxid to the values in pagefrz->FreezePageRelfrozenXid and
+ * pagefrz->FreezePageRelminMxid. Otherwise, it is only safe to advance to
+ * the values in pagefrz->NoFreezePage[RelfrozenXid|RelminMxid]
+ */
+ if (pagefrz)
{
- START_CRIT_SECTION();
+ if (presult->nfrozen > 0)
+ {
+ presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
+ }
+ else
+ {
+ presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
+ presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
+ }
+ }
+}
- Assert(presult->nfrozen > 0);
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+static void
+log_heap_prune_and_freeze(Relation relation, Buffer buffer,
+ PruneState *prstate, PruneFreezeResult *presult)
+{
+ xl_heap_prune xlrec;
+ XLogRecPtr recptr;
- MarkBufferDirty(buffer);
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+ bool do_freeze = presult->nfrozen > 0;
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
+ xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+ xlrec.nredirected = prstate->nredirected;
+ xlrec.ndead = prstate->ndead;
+ xlrec.nunused = prstate->nunused;
+ xlrec.nplans = 0;
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(prstate.frozen, presult->nfrozen, plans, offsets);
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions on
+ * the standby older than the youngest xmax of the most recently removed
+ * tuple this record will prune will conflict. If this record will freeze
+ * tuples, any transactions on the standby with xids older than the
+ * youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate->snapshotConflictHorizon,
+ presult->frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate->snapshotConflictHorizon;
- xlrec.snapshotConflictHorizon = presult->frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
+ /*
+ * Prepare deduplicated representation for use in WAL record Destructively
+ * sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(prstate->frozen,
+ presult->nfrozen, plans, offsets);
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+ XLogBeginInsert();
+ XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
- XLogRegisterBufData(0, (char *) offsets,
- presult->nfrozen * sizeof(OffsetNumber));
+ XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ /*
+ * The OffsetNumber arrays are not actually in the buffer, but we pretend
+ * that they are. When XLogInsert stores the whole buffer, the offset
+ * arrays need not be stored too.
+ */
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
- PageSetLSN(page, recptr);
- }
+ if (prstate->nredirected > 0)
+ XLogRegisterBufData(0, (char *) prstate->redirected,
+ prstate->nredirected *
+ sizeof(OffsetNumber) * 2);
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
+ if (prstate->ndead > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowdead,
+ prstate->ndead * sizeof(OffsetNumber));
- /* Caller won't update new_relfrozenxid and new_relminmxid */
- if (!pagefrz)
- return;
+ if (prstate->nunused > 0)
+ XLogRegisterBufData(0, (char *) prstate->nowunused,
+ prstate->nunused * sizeof(OffsetNumber));
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) offsets,
+ presult->nfrozen * sizeof(OffsetNumber));
- /*
- * If we will freeze tuples on the page or, even if we don't freeze tuples
- * on the page, if we will set the page all-frozen in the visibility map,
- * we can advance relfrozenxid and relminmxid to the values in
- * pagefrz->FreezePageRelfrozenXid and pagefrz->FreezePageRelminMxid.
- */
- if (presult->all_frozen || presult->nfrozen > 0)
- {
- presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
- }
- else
- {
- presult->new_relfrozenxid = pagefrz->NoFreezePageRelfrozenXid;
- presult->new_relminmxid = pagefrz->NoFreezePageRelminMxid;
- }
-}
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
+ PageSetLSN(BufferGetPage(buffer), recptr);
+}
/*
* Perform visibility checks for heap pruning.
diff --git a/src/backend/access/rmgrdesc/heapdesc.c b/src/backend/access/rmgrdesc/heapdesc.c
index 36a3d83c8c2..9f0a0341d40 100644
--- a/src/backend/access/rmgrdesc/heapdesc.c
+++ b/src/backend/access/rmgrdesc/heapdesc.c
@@ -179,43 +179,67 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
{
xl_heap_prune *xlrec = (xl_heap_prune *) rec;
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nredirected: %u, ndead: %u, isCatalogRel: %c",
+ appendStringInfo(buf, "snapshotConflictHorizon: %u, isCatalogRel: %c",
xlrec->snapshotConflictHorizon,
- xlrec->nredirected,
- xlrec->ndead,
xlrec->isCatalogRel ? 'T' : 'F');
if (XLogRecHasBlockData(record, 0))
{
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
+ int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0,
- &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
- end = (OffsetNumber *) ((char *) redirected + datalen);
- nowdead = redirected + (nredirected * 2);
- nowunused = nowdead + xlrec->ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ ndead = xlrec->ndead;
+ nunused = xlrec->nunused;
- appendStringInfo(buf, ", nunused: %d", nunused);
-
- appendStringInfoString(buf, ", redirected:");
- array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
- nredirected, &redirect_elem_desc, NULL);
- appendStringInfoString(buf, ", dead:");
- array_desc(buf, nowdead, sizeof(OffsetNumber), xlrec->ndead,
- &offset_elem_desc, NULL);
- appendStringInfoString(buf, ", unused:");
- array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
- &offset_elem_desc, NULL);
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
+ nowdead = redirected + (nredirected * 2);
+ nowunused = nowdead + ndead;
+ frz_offsets = nowunused + nunused;
+
+ appendStringInfo(buf, ", nredirected: %u, ndead: %u, nunused: %u, nplans: %u,",
+ nredirected,
+ ndead,
+ nunused,
+ nplans);
+
+ if (nredirected > 0)
+ {
+ appendStringInfoString(buf, ", redirected:");
+ array_desc(buf, redirected, sizeof(OffsetNumber) * 2,
+ nredirected, &redirect_elem_desc, NULL);
+ }
+
+ if (ndead > 0)
+ {
+ appendStringInfoString(buf, ", dead:");
+ array_desc(buf, nowdead, sizeof(OffsetNumber), ndead,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nunused > 0)
+ {
+ appendStringInfoString(buf, ", unused:");
+ array_desc(buf, nowunused, sizeof(OffsetNumber), nunused,
+ &offset_elem_desc, NULL);
+ }
+
+ if (nplans > 0)
+ {
+ appendStringInfoString(buf, ", plans:");
+ array_desc(buf, plans, sizeof(xl_heap_freeze_plan), nplans,
+ &plan_elem_desc, &frz_offsets);
+ }
}
}
else if (info == XLOG_HEAP2_VACUUM)
@@ -235,28 +259,6 @@ heap2_desc(StringInfo buf, XLogReaderState *record)
&offset_elem_desc, NULL);
}
}
- else if (info == XLOG_HEAP2_FREEZE_PAGE)
- {
- xl_heap_freeze_page *xlrec = (xl_heap_freeze_page *) rec;
-
- appendStringInfo(buf, "snapshotConflictHorizon: %u, nplans: %u, isCatalogRel: %c",
- xlrec->snapshotConflictHorizon, xlrec->nplans,
- xlrec->isCatalogRel ? 'T' : 'F');
-
- if (XLogRecHasBlockData(record, 0))
- {
- xl_heap_freeze_plan *plans;
- OffsetNumber *offsets;
-
- plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, NULL);
- offsets = (OffsetNumber *) ((char *) plans +
- (xlrec->nplans *
- sizeof(xl_heap_freeze_plan)));
- appendStringInfoString(buf, ", plans:");
- array_desc(buf, plans, sizeof(xl_heap_freeze_plan), xlrec->nplans,
- &plan_elem_desc, &offsets);
- }
- }
else if (info == XLOG_HEAP2_VISIBLE)
{
xl_heap_visible *xlrec = (xl_heap_visible *) rec;
@@ -361,9 +363,6 @@ heap2_identify(uint8 info)
case XLOG_HEAP2_VACUUM:
id = "VACUUM";
break;
- case XLOG_HEAP2_FREEZE_PAGE:
- id = "FREEZE_PAGE";
- break;
case XLOG_HEAP2_VISIBLE:
id = "VISIBLE";
break;
diff --git a/src/backend/replication/logical/decode.c b/src/backend/replication/logical/decode.c
index e5ab7b78b78..f77051572fd 100644
--- a/src/backend/replication/logical/decode.c
+++ b/src/backend/replication/logical/decode.c
@@ -445,7 +445,6 @@ heap2_decode(LogicalDecodingContext *ctx, XLogRecordBuffer *buf)
* Everything else here is just low level physical stuff we're not
* interested in.
*/
- case XLOG_HEAP2_FREEZE_PAGE:
case XLOG_HEAP2_PRUNE:
case XLOG_HEAP2_VACUUM:
case XLOG_HEAP2_VISIBLE:
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..fe4a8ff0620 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -53,11 +53,10 @@
#define XLOG_HEAP2_REWRITE 0x00
#define XLOG_HEAP2_PRUNE 0x10
#define XLOG_HEAP2_VACUUM 0x20
-#define XLOG_HEAP2_FREEZE_PAGE 0x30
-#define XLOG_HEAP2_VISIBLE 0x40
-#define XLOG_HEAP2_MULTI_INSERT 0x50
-#define XLOG_HEAP2_LOCK_UPDATED 0x60
-#define XLOG_HEAP2_NEW_CID 0x70
+#define XLOG_HEAP2_VISIBLE 0x30
+#define XLOG_HEAP2_MULTI_INSERT 0x40
+#define XLOG_HEAP2_LOCK_UPDATED 0x50
+#define XLOG_HEAP2_NEW_CID 0x60
/*
* xl_heap_insert/xl_heap_multi_insert flag values, 8 bits are available.
@@ -226,28 +225,65 @@ typedef struct xl_heap_update
#define SizeOfHeapUpdate (offsetof(xl_heap_update, new_offnum) + sizeof(OffsetNumber))
+/*
+ * This struct represents a 'freeze plan', which describes how to freeze a
+ * group of one or more heap tuples (appears in xl_heap_prune record)
+ */
+/* 0x01 was XLH_FREEZE_XMIN */
+#define XLH_FREEZE_XVAC 0x02
+#define XLH_INVALID_XVAC 0x04
+
+typedef struct xl_heap_freeze_plan
+{
+ TransactionId xmax;
+ uint16 t_infomask2;
+ uint16 t_infomask;
+ uint8 frzflags;
+
+ /* Length of individual page offset numbers array for this plan */
+ uint16 ntuples;
+} xl_heap_freeze_plan;
+
+/*
+ * As of Postgres 17, XLOG_HEAP2_PRUNE records replace
+ * XLOG_HEAP2_FREEZE_PAGE records.
+ */
+
/*
* This is what we need to know about page pruning (both during VACUUM and
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
- * Acquires a full cleanup lock.
+ * Acquires a full cleanup lock if heap_page_prune_execute() must be called
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*--------------------------------------------------------------------
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ *--------------------------------------------------------------------
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
@@ -315,47 +351,6 @@ typedef struct xl_heap_inplace
#define SizeOfHeapInplace (offsetof(xl_heap_inplace, offnum) + sizeof(OffsetNumber))
-/*
- * This struct represents a 'freeze plan', which describes how to freeze a
- * group of one or more heap tuples (appears in xl_heap_freeze_page record)
- */
-/* 0x01 was XLH_FREEZE_XMIN */
-#define XLH_FREEZE_XVAC 0x02
-#define XLH_INVALID_XVAC 0x04
-
-typedef struct xl_heap_freeze_plan
-{
- TransactionId xmax;
- uint16 t_infomask2;
- uint16 t_infomask;
- uint8 frzflags;
-
- /* Length of individual page offset numbers array for this plan */
- uint16 ntuples;
-} xl_heap_freeze_plan;
-
-/*
- * This is what we need to know about a block being frozen during vacuum
- *
- * Backup block 0's data contains an array of xl_heap_freeze_plan structs
- * (with nplans elements), followed by one or more page offset number arrays.
- * Each such page offset number array corresponds to a single freeze plan
- * (REDO routine freezes corresponding heap tuples using freeze plan).
- */
-typedef struct xl_heap_freeze_page
-{
- TransactionId snapshotConflictHorizon;
- uint16 nplans;
- bool isCatalogRel; /* to handle recovery conflict during logical
- * decoding on standby */
-
- /*
- * In payload of blk 0 : FREEZE PLANS and OFFSET NUMBER ARRAY
- */
-} xl_heap_freeze_page;
-
-#define SizeOfHeapFreezePage (offsetof(xl_heap_freeze_page, isCatalogRel) + sizeof(bool))
-
/*
* This is what we need to know about setting a visibility map bit
*
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0014-Vacuum-second-pass-emits-XLOG_HEAP2_PRUNE-record.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v2 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca64c45d8a3..70d35a21e98 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -605,6 +605,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -615,10 +618,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -630,6 +660,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -643,56 +677,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v2 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ca64c45d8a3..70d35a21e98 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -605,6 +605,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -615,10 +618,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -630,6 +660,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -643,56 +677,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v3 12/17] Merge prune and freeze records
@ 2024-01-07 22:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-01-07 22:55 UTC (permalink / raw)
When there are both tuples to prune and freeze on a page, emit a single,
combined prune record containing the offsets for pruning and the freeze
plans and offsets for freezing. This will reduce the number of WAL
records emitted.
---
src/backend/access/heap/heapam.c | 42 ++++++++++++--
src/backend/access/heap/pruneheap.c | 85 +++++++++++++----------------
src/include/access/heapam_xlog.h | 20 +++++--
3 files changed, 90 insertions(+), 57 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a3691584c55..a8f35eba3c9 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8803,24 +8803,28 @@ heap_xlog_prune(XLogReaderState *record)
if (action == BLK_NEEDS_REDO)
{
Page page = (Page) BufferGetPage(buffer);
- OffsetNumber *end;
OffsetNumber *redirected;
OffsetNumber *nowdead;
OffsetNumber *nowunused;
int nredirected;
int ndead;
int nunused;
+ int nplans;
Size datalen;
+ xl_heap_freeze_plan *plans;
+ OffsetNumber *frz_offsets;
+ int curoff = 0;
- redirected = (OffsetNumber *) XLogRecGetBlockData(record, 0, &datalen);
-
+ nplans = xlrec->nplans;
nredirected = xlrec->nredirected;
ndead = xlrec->ndead;
- end = (OffsetNumber *) ((char *) redirected + datalen);
+ nunused = xlrec->nunused;
+
+ plans = (xl_heap_freeze_plan *) XLogRecGetBlockData(record, 0, &datalen);
+ redirected = (OffsetNumber *) &plans[nplans];
nowdead = redirected + (nredirected * 2);
nowunused = nowdead + ndead;
- nunused = (end - nowunused);
- Assert(nunused >= 0);
+ frz_offsets = nowunused + nunused;
/* Update all line pointers per the record, and repair fragmentation */
heap_page_prune_execute(buffer,
@@ -8828,6 +8832,32 @@ heap_xlog_prune(XLogReaderState *record)
nowdead, ndead,
nowunused, nunused);
+ for (int p = 0; p < nplans; p++)
+ {
+ HeapTupleFreeze frz;
+
+ /*
+ * Convert freeze plan representation from WAL record into
+ * per-tuple format used by heap_execute_freeze_tuple
+ */
+ frz.xmax = plans[p].xmax;
+ frz.t_infomask2 = plans[p].t_infomask2;
+ frz.t_infomask = plans[p].t_infomask;
+ frz.frzflags = plans[p].frzflags;
+ frz.offset = InvalidOffsetNumber; /* unused, but be tidy */
+
+ for (int i = 0; i < plans[p].ntuples; i++)
+ {
+ OffsetNumber offset = frz_offsets[curoff++];
+ ItemId lp;
+ HeapTupleHeader tuple;
+
+ lp = PageGetItemId(page, offset);
+ tuple = (HeapTupleHeader) PageGetItem(page, lp);
+ heap_execute_freeze_tuple(tuple, &frz);
+ }
+ }
+
/*
* Note: we don't worry about updating the page's prunability hints.
* At worst this will cause an extra prune cycle to occur soon.
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index d77270ad0d6..994cf75c54e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -619,6 +619,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
PageClearFull(page);
+ if (do_freeze)
+ heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
MarkBufferDirty(buffer);
/*
@@ -629,10 +632,37 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
xl_heap_prune xlrec;
XLogRecPtr recptr;
+ xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
+
xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
xlrec.nredirected = prstate.nredirected;
xlrec.ndead = prstate.ndead;
+ xlrec.nunused = prstate.nunused;
+ xlrec.nplans = 0;
+
+ /*
+ * The snapshotConflictHorizon for the whole record should be the most
+ * conservative of all the horizons calculated for any of the possible
+ * modifications. If this record will prune tuples, any transactions
+ * on the standby older than the youngest xmax of the most recently
+ * removed tuple this record will prune will conflict. If this record
+ * will freeze tuples, any transactions on the standby with xids older
+ * than the youngest tuple this record will freeze will conflict.
+ */
+ if (do_freeze)
+ xlrec.snapshotConflictHorizon = Max(prstate.snapshotConflictHorizon,
+ frz_conflict_horizon);
+ else
+ xlrec.snapshotConflictHorizon = prstate.snapshotConflictHorizon;
+
+ /*
+ * Prepare deduplicated representation for use in WAL record
+ * Destructively sorts tuples array in-place.
+ */
+ if (do_freeze)
+ xlrec.nplans = heap_log_freeze_plan(frozen,
+ presult->nfrozen, plans, offsets);
XLogBeginInsert();
XLogRegisterData((char *) &xlrec, SizeOfHeapPrune);
@@ -644,6 +674,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* pretend that they are. When XLogInsert stores the whole buffer,
* the offset arrays need not be stored too.
*/
+ if (xlrec.nplans > 0)
+ XLogRegisterBufData(0, (char *) plans,
+ xlrec.nplans * sizeof(xl_heap_freeze_plan));
+
if (prstate.nredirected > 0)
XLogRegisterBufData(0, (char *) prstate.redirected,
prstate.nredirected *
@@ -657,56 +691,13 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
XLogRegisterBufData(0, (char *) prstate.nowunused,
prstate.nunused * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
-
- PageSetLSN(BufferGetPage(buffer), recptr);
- }
-
- if (do_freeze)
- {
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- {
- xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
- OffsetNumber offsets[MaxHeapTuplesPerPage];
- int nplans;
- xl_heap_freeze_page xlrec;
- XLogRecPtr recptr;
-
- /*
- * Prepare deduplicated representation for use in WAL record
- * Destructively sorts tuples array in-place.
- */
- nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
-
- xlrec.snapshotConflictHorizon = frz_conflict_horizon;
- xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
- xlrec.nplans = nplans;
-
- XLogBeginInsert();
- XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
-
- /*
- * The freeze plan array and offset array are not actually in the
- * buffer, but pretend that they are. When XLogInsert stores the
- * whole buffer, the arrays need not be stored too.
- */
- XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
- XLogRegisterBufData(0, (char *) plans,
- nplans * sizeof(xl_heap_freeze_plan));
+ if (xlrec.nplans > 0)
XLogRegisterBufData(0, (char *) offsets,
presult->nfrozen * sizeof(OffsetNumber));
- recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+ recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_PRUNE);
- PageSetLSN(page, recptr);
- }
+ PageSetLSN(BufferGetPage(buffer), recptr);
}
END_CRIT_SECTION();
diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h
index 6488dad5e64..22f236bb52a 100644
--- a/src/include/access/heapam_xlog.h
+++ b/src/include/access/heapam_xlog.h
@@ -231,23 +231,35 @@ typedef struct xl_heap_update
* during opportunistic pruning)
*
* The array of OffsetNumbers following the fixed part of the record contains:
+ * * for each freeze plan: the freeze plan
* * for each redirected item: the item offset, then the offset redirected to
* * for each now-dead item: the item offset
* * for each now-unused item: the item offset
- * The total number of OffsetNumbers is therefore 2*nredirected+ndead+nunused.
- * Note that nunused is not explicitly stored, but may be found by reference
- * to the total record length.
+ * * for each tuple frozen by the freeze plans: the offset of the item corresponding to that tuple
+ * The total number of OffsetNumbers is therefore
+ * (2*nredirected) + ndead + nunused + (sum[plan.ntuples for plan in plans])
*
* Acquires a full cleanup lock.
*/
typedef struct xl_heap_prune
{
TransactionId snapshotConflictHorizon;
+ uint16 nplans;
uint16 nredirected;
uint16 ndead;
+ uint16 nunused;
bool isCatalogRel; /* to handle recovery conflict during logical
* decoding on standby */
- /* OFFSET NUMBERS are in the block reference 0 */
+ /*
+ * OFFSET NUMBERS and freeze plans are in the block reference 0 in the
+ * following order:
+ *
+ * * xl_heap_freeze_plan plans[nplans];
+ * * OffsetNumber redirected[2 * nredirected];
+ * * OffsetNumber nowdead[ndead];
+ * * OffsetNumber nowunused[nunused];
+ * * OffsetNumber frz_offsets[...];
+ */
} xl_heap_prune;
#define SizeOfHeapPrune (offsetof(xl_heap_prune, isCatalogRel) + sizeof(bool))
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v7 12/16] Merge prune and freeze records
@ 2024-03-26 13:37 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-03-26 13:37 UTC (permalink / raw)
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
---
src/backend/access/heap/heapam.c | 2 -
src/backend/access/heap/pruneheap.c | 215 +++++++++++++++-------------
2 files changed, 114 insertions(+), 103 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 1c1785994b1..5d8f183085d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6731,8 +6731,6 @@ heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
htup = (HeapTupleHeader) PageGetItem(page, itemid);
heap_execute_freeze_tuple(htup, frz);
}
-
- MarkBufferDirty(buffer);
}
/*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 8914d4bf5c8..db8a182a197 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -249,9 +249,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
- bool whole_page_freezable;
+ bool do_hint;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -464,10 +463,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -517,16 +515,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
-
- /* Is the whole page freezable? And is there something to freeze */
- whole_page_freezable = all_visible_except_removable &&
- presult->all_frozen;
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
/*
* Freeze the page when heap_prepare_freeze_tuple indicates that at least
@@ -539,46 +537,57 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
- if (do_freeze)
+ do_freeze = false;
+ if (pagefrz)
{
- heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ /* Is the whole page freezable? And is there something to freeze? */
+ bool whole_page_freezable = all_visible_except_removable &&
+ presult->all_frozen;
- /*
- * 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
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
{
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
}
}
- /* Any error while applying the changes is critical */
- START_CRIT_SECTION();
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
+ if (do_freeze)
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
- /* Have we found any prunable items? */
- if (do_prune)
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
{
/*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
+ * If we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- heap_page_prune_execute(buffer, false,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
+ }
+
+ /* Any error while applying the changes is critical */
+ START_CRIT_SECTION();
+ if (do_hint)
+ {
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -586,12 +595,52 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+
+ if (do_prune || do_freeze)
+ {
+ /* Apply the planned item changes, then repair page fragmentation. */
+ if (do_prune)
+ {
+ heap_page_prune_execute(buffer, false,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+
+ if (do_freeze)
+ {
+ /*
+ * 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);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
+
MarkBufferDirty(buffer);
/*
@@ -599,72 +648,35 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
if (RelationNeedsWAL(relation))
{
+ /*
+ * The snapshotConflictHorizon for the whole record should be the
+ * most conservative of all the horizons calculated for any of the
+ * possible modifications. If this record will prune tuples, any
+ * transactions on the standby older than the youngest xmax of the
+ * most recently removed tuple this record will prune will
+ * conflict. If this record will freeze tuples, any transactions
+ * on the standby with xids older than the youngest tuple this
+ * record will freeze will conflict.
+ */
+ TransactionId conflict_xid;
+
+ if (TransactionIdFollows(frz_conflict_horizon, prstate.latest_xid_removed))
+ conflict_xid = frz_conflict_horizon;
+ else
+ conflict_xid = prstate.latest_xid_removed;
+
log_heap_prune_and_freeze(relation, buffer,
- prstate.latest_xid_removed,
+ conflict_xid,
true, reason,
- NULL, 0,
+ prstate.frozen, presult->nfrozen,
prstate.redirected, prstate.nredirected,
prstate.nowdead, prstate.ndead,
prstate.nowunused, prstate.nunused);
}
}
- else
- {
- /*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
- */
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
- }
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
- {
- START_CRIT_SECTION();
-
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- log_heap_prune_and_freeze(relation, buffer,
- frz_conflict_horizon, false, reason,
- prstate.frozen, presult->nfrozen,
- NULL, 0, /* redirected */
- NULL, 0, /* dead */
- NULL, 0); /* unused */
-
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
-
/*
* For callers planning to update the visibility map, the conflict horizon
* for that record must be the newest xmin on the page. However, if the
@@ -681,9 +693,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* tuples on the page, if we will set the page all-frozen in the
* visibility map, we can advance relfrozenxid and relminmxid to the
* values in pagefrz->FreezePageRelfrozenXid and
- * pagefrz->FreezePageRelminMxid.
+ * pagefrz->FreezePageRelminMxid. MFIXME: which one should be pick if
+ * presult->nfrozen == 0 and presult->all_frozen = True.
*/
- if (presult->all_frozen || presult->nfrozen > 0)
+ if (presult->nfrozen > 0)
{
presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v9 12/21] Merge prune and freeze records
@ 2024-03-26 13:37 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-03-26 13:37 UTC (permalink / raw)
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
---
src/backend/access/heap/heapam.c | 2 -
src/backend/access/heap/pruneheap.c | 215 +++++++++++++++-------------
2 files changed, 114 insertions(+), 103 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 41c1c7d286f..aefc0be0dd3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6836,8 +6836,6 @@ heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
htup = (HeapTupleHeader) PageGetItem(page, itemid);
heap_execute_freeze_tuple(htup, frz);
}
-
- MarkBufferDirty(buffer);
}
/*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 5d8c881c2fc..6085fd1a8f9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -249,9 +249,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
- bool whole_page_freezable;
+ bool do_hint;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -464,10 +463,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -517,16 +515,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
-
- /* Is the whole page freezable? And is there something to freeze */
- whole_page_freezable = all_visible_except_removable &&
- presult->all_frozen;
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
/*
* Freeze the page when heap_prepare_freeze_tuple indicates that at least
@@ -539,46 +537,57 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
- if (do_freeze)
+ do_freeze = false;
+ if (pagefrz)
{
- heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ /* Is the whole page freezable? And is there something to freeze? */
+ bool whole_page_freezable = all_visible_except_removable &&
+ presult->all_frozen;
- /*
- * 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
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
{
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
}
}
- /* Any error while applying the changes is critical */
- START_CRIT_SECTION();
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
+ if (do_freeze)
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
- /* Have we found any prunable items? */
- if (do_prune)
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
{
/*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
+ * If we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- heap_page_prune_execute(buffer, false,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
+ }
+
+ /* Any error while applying the changes is critical */
+ START_CRIT_SECTION();
+ if (do_hint)
+ {
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -586,12 +595,52 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+
+ if (do_prune || do_freeze)
+ {
+ /* Apply the planned item changes, then repair page fragmentation. */
+ if (do_prune)
+ {
+ heap_page_prune_execute(buffer, false,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+
+ if (do_freeze)
+ {
+ /*
+ * 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);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
+
MarkBufferDirty(buffer);
/*
@@ -599,72 +648,35 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
if (RelationNeedsWAL(relation))
{
+ /*
+ * The snapshotConflictHorizon for the whole record should be the
+ * most conservative of all the horizons calculated for any of the
+ * possible modifications. If this record will prune tuples, any
+ * transactions on the standby older than the youngest xmax of the
+ * most recently removed tuple this record will prune will
+ * conflict. If this record will freeze tuples, any transactions
+ * on the standby with xids older than the youngest tuple this
+ * record will freeze will conflict.
+ */
+ TransactionId conflict_xid;
+
+ if (TransactionIdFollows(frz_conflict_horizon, prstate.latest_xid_removed))
+ conflict_xid = frz_conflict_horizon;
+ else
+ conflict_xid = prstate.latest_xid_removed;
+
log_heap_prune_and_freeze(relation, buffer,
- prstate.latest_xid_removed,
+ conflict_xid,
true, reason,
- NULL, 0,
+ prstate.frozen, presult->nfrozen,
prstate.redirected, prstate.nredirected,
prstate.nowdead, prstate.ndead,
prstate.nowunused, prstate.nunused);
}
}
- else
- {
- /*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
- */
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
- }
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
- {
- START_CRIT_SECTION();
-
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- log_heap_prune_and_freeze(relation, buffer,
- frz_conflict_horizon, false, reason,
- prstate.frozen, presult->nfrozen,
- NULL, 0, /* redirected */
- NULL, 0, /* dead */
- NULL, 0); /* unused */
-
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
-
/*
* For callers planning to update the visibility map, the conflict horizon
* for that record must be the newest xmin on the page. However, if the
@@ -681,9 +693,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* tuples on the page, if we will set the page all-frozen in the
* visibility map, we can advance relfrozenxid and relminmxid to the
* values in pagefrz->FreezePageRelfrozenXid and
- * pagefrz->FreezePageRelminMxid.
+ * pagefrz->FreezePageRelminMxid. MFIXME: which one should be pick if
+ * presult->nfrozen == 0 and presult->all_frozen = True.
*/
- if (presult->all_frozen || presult->nfrozen > 0)
+ if (presult->nfrozen > 0)
{
presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH v7 12/16] Merge prune and freeze records
@ 2024-03-26 13:37 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Melanie Plageman @ 2024-03-26 13:37 UTC (permalink / raw)
When both pruning and freezing is done, this means a single, combined
WAL record is emitted for both operations. This will reduce the number
of WAL records emitted.
When there are only tuples to freeze present, we can avoid taking a full
cleanup lock when replaying the record.
---
src/backend/access/heap/heapam.c | 2 -
src/backend/access/heap/pruneheap.c | 215 +++++++++++++++-------------
2 files changed, 114 insertions(+), 103 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 1c1785994b1..5d8f183085d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -6731,8 +6731,6 @@ heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
htup = (HeapTupleHeader) PageGetItem(page, itemid);
heap_execute_freeze_tuple(htup, frz);
}
-
- MarkBufferDirty(buffer);
}
/*
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 8914d4bf5c8..db8a182a197 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -249,9 +249,8 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
bool do_freeze;
bool all_visible_except_removable;
bool do_prune;
- bool whole_page_freezable;
+ bool do_hint;
bool hint_bit_fpi;
- bool prune_fpi = false;
int64 fpi_before = pgWalUsage.wal_fpi;
/*
@@ -464,10 +463,9 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
/*
* If checksums are enabled, heap_prune_satisfies_vacuum() may have caused
- * an FPI to be emitted. Then reset fpi_before for no prune case.
+ * an FPI to be emitted.
*/
hint_bit_fpi = fpi_before != pgWalUsage.wal_fpi;
- fpi_before = pgWalUsage.wal_fpi;
/*
* For vacuum, if the whole page will become frozen, we consider
@@ -517,16 +515,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
prstate.ndead > 0 ||
prstate.nunused > 0;
+ /* Record number of newly-set-LP_DEAD items for caller */
+ presult->nnewlpdead = prstate.ndead;
+
/*
- * Only incur overhead of checking if we will do an FPI if we might use
- * the information.
+ * Even if we don't prune anything, if we found a new value for the
+ * pd_prune_xid field or the page was marked full, we will update the hint
+ * bit.
*/
- if (do_prune && pagefrz)
- prune_fpi = XLogCheckBufferNeedsBackup(buffer);
-
- /* Is the whole page freezable? And is there something to freeze */
- whole_page_freezable = all_visible_except_removable &&
- presult->all_frozen;
+ do_hint = ((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
+ PageIsFull(page);
/*
* Freeze the page when heap_prepare_freeze_tuple indicates that at least
@@ -539,46 +537,57 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* opportunistic freeze heuristic must be improved; however, for now, try
* to approximate it.
*/
- do_freeze = pagefrz &&
- (pagefrz->freeze_required ||
- (whole_page_freezable && presult->nfrozen > 0 && (prune_fpi || hint_bit_fpi)));
- if (do_freeze)
+ do_freeze = false;
+ if (pagefrz)
{
- heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
+ /* Is the whole page freezable? And is there something to freeze? */
+ bool whole_page_freezable = all_visible_except_removable &&
+ presult->all_frozen;
- /*
- * 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
+ if (pagefrz->freeze_required)
+ do_freeze = true;
+ else if (whole_page_freezable && presult->nfrozen > 0)
{
- /* Avoids false conflicts when hot_standby_feedback in use */
- frz_conflict_horizon = pagefrz->cutoffs->OldestXmin;
- TransactionIdRetreat(frz_conflict_horizon);
+ /*
+ * Freezing would make the page all-frozen. In this case, we will
+ * freeze if we have already emitted an FPI or will do so anyway.
+ * Be sure only to incur the overhead of checking if we will do an
+ * FPI if we may use that information.
+ */
+ if (hint_bit_fpi ||
+ ((do_prune || do_hint) && XLogCheckBufferNeedsBackup(buffer)))
+ {
+ do_freeze = true;
+ }
}
}
- /* Any error while applying the changes is critical */
- START_CRIT_SECTION();
+ /*
+ * Validate the tuples we are considering freezing. We do this even if
+ * pruning and hint bit setting have not emitted an FPI so far because we
+ * still may emit an FPI while setting the page hint bit later. But we
+ * want to avoid doing the pre-freeze checks in a critical section.
+ */
+ if (do_freeze)
+ heap_pre_freeze_checks(buffer, prstate.frozen, presult->nfrozen);
- /* Have we found any prunable items? */
- if (do_prune)
+ if (!do_freeze && (!pagefrz || !presult->all_frozen || presult->nfrozen > 0))
{
/*
- * Apply the planned item changes, then repair page fragmentation, and
- * update the page's hint bit about whether it has free line pointers.
+ * If we will neither freeze tuples on the page nor set the page all
+ * frozen in the visibility map, the page is not all-frozen and there
+ * will be no newly frozen tuples.
*/
- heap_page_prune_execute(buffer, false,
- prstate.redirected, prstate.nredirected,
- prstate.nowdead, prstate.ndead,
- prstate.nowunused, prstate.nunused);
+ presult->all_frozen = false;
+ presult->nfrozen = 0; /* avoid miscounts in instrumentation */
+ }
+
+ /* Any error while applying the changes is critical */
+ START_CRIT_SECTION();
+ if (do_hint)
+ {
/*
* Update the page's pd_prune_xid field to either zero, or the lowest
* XID of any soon-prunable tuple.
@@ -586,12 +595,52 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
/*
- * Also clear the "page is full" flag, since there's no point in
- * repeating the prune/defrag process until something else happens to
- * the page.
+ * Clear the "page is full" flag if it is set since there's no point
+ * in repeating the prune/defrag process until something else happens
+ * to the page.
*/
PageClearFull(page);
+ /*
+ * We only needed to update pd_prune_xid and clear the page-is-full
+ * hint bit, this is a non-WAL-logged hint. If we will also freeze or
+ * prune the page, we will mark the buffer dirty below.
+ */
+ if (!do_freeze && !do_prune)
+ MarkBufferDirtyHint(buffer, true);
+ }
+
+ if (do_prune || do_freeze)
+ {
+ /* Apply the planned item changes, then repair page fragmentation. */
+ if (do_prune)
+ {
+ heap_page_prune_execute(buffer, false,
+ prstate.redirected, prstate.nredirected,
+ prstate.nowdead, prstate.ndead,
+ prstate.nowunused, prstate.nunused);
+ }
+
+ if (do_freeze)
+ {
+ /*
+ * 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);
+ }
+ heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
+ }
+
MarkBufferDirty(buffer);
/*
@@ -599,72 +648,35 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
*/
if (RelationNeedsWAL(relation))
{
+ /*
+ * The snapshotConflictHorizon for the whole record should be the
+ * most conservative of all the horizons calculated for any of the
+ * possible modifications. If this record will prune tuples, any
+ * transactions on the standby older than the youngest xmax of the
+ * most recently removed tuple this record will prune will
+ * conflict. If this record will freeze tuples, any transactions
+ * on the standby with xids older than the youngest tuple this
+ * record will freeze will conflict.
+ */
+ TransactionId conflict_xid;
+
+ if (TransactionIdFollows(frz_conflict_horizon, prstate.latest_xid_removed))
+ conflict_xid = frz_conflict_horizon;
+ else
+ conflict_xid = prstate.latest_xid_removed;
+
log_heap_prune_and_freeze(relation, buffer,
- prstate.latest_xid_removed,
+ conflict_xid,
true, reason,
- NULL, 0,
+ prstate.frozen, presult->nfrozen,
prstate.redirected, prstate.nredirected,
prstate.nowdead, prstate.ndead,
prstate.nowunused, prstate.nunused);
}
}
- else
- {
- /*
- * If we didn't prune anything, but have found a new value for the
- * pd_prune_xid field, update it and mark the buffer dirty. This is
- * treated as a non-WAL-logged hint.
- *
- * Also clear the "page is full" flag if it is set, since there's no
- * point in repeating the prune/defrag process until something else
- * happens to the page.
- */
- if (((PageHeader) page)->pd_prune_xid != prstate.new_prune_xid ||
- PageIsFull(page))
- {
- ((PageHeader) page)->pd_prune_xid = prstate.new_prune_xid;
- PageClearFull(page);
- MarkBufferDirtyHint(buffer, true);
- }
- }
END_CRIT_SECTION();
- /* Record number of newly-set-LP_DEAD items for caller */
- presult->nnewlpdead = prstate.ndead;
-
- if (do_freeze)
- {
- START_CRIT_SECTION();
-
- Assert(presult->nfrozen > 0);
-
- heap_freeze_prepared_tuples(buffer, prstate.frozen, presult->nfrozen);
-
- MarkBufferDirty(buffer);
-
- /* Now WAL-log freezing if necessary */
- if (RelationNeedsWAL(relation))
- log_heap_prune_and_freeze(relation, buffer,
- frz_conflict_horizon, false, reason,
- prstate.frozen, presult->nfrozen,
- NULL, 0, /* redirected */
- NULL, 0, /* dead */
- NULL, 0); /* unused */
-
- END_CRIT_SECTION();
- }
- else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
- {
- /*
- * If we will neither freeze tuples on the page nor set the page all
- * frozen in the visibility map, the page is not all frozen and there
- * will be no newly frozen tuples.
- */
- presult->all_frozen = false;
- presult->nfrozen = 0; /* avoid miscounts in instrumentation */
- }
-
/*
* For callers planning to update the visibility map, the conflict horizon
* for that record must be the newest xmin on the page. However, if the
@@ -681,9 +693,10 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
* tuples on the page, if we will set the page all-frozen in the
* visibility map, we can advance relfrozenxid and relminmxid to the
* values in pagefrz->FreezePageRelfrozenXid and
- * pagefrz->FreezePageRelminMxid.
+ * pagefrz->FreezePageRelminMxid. MFIXME: which one should be pick if
+ * presult->nfrozen == 0 and presult->all_frozen = True.
*/
- if (presult->all_frozen || presult->nfrozen > 0)
+ if (presult->nfrozen > 0)
{
presult->new_relfrozenxid = pagefrz->FreezePageRelfrozenXid;
presult->new_relminmxid = pagefrz->FreezePageRelminMxid;
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0013-Set-hastup-in-heap_page_prune.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
end of thread, other threads:[~2024-03-26 13:37 UTC | newest]
Thread overview: 29+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-17 22:58 Re: O(n) tasks cause lengthy startups and checkpoints Nathan Bossart <[email protected]>
2022-02-17 23:12 ` Andres Freund <[email protected]>
2022-02-18 16:44 ` Nathan Bossart <[email protected]>
2022-02-18 20:51 ` Nathan Bossart <[email protected]>
2022-03-17 23:07 ` Nathan Bossart <[email protected]>
2022-06-23 11:58 ` Simon Riggs <[email protected]>
2022-06-23 13:46 ` Robert Haas <[email protected]>
2022-06-23 17:13 ` Simon Riggs <[email protected]>
2022-06-23 17:15 ` Nathan Bossart <[email protected]>
2022-06-24 10:45 ` Simon Riggs <[email protected]>
2022-07-02 22:05 ` Nathan Bossart <[email protected]>
2022-07-02 22:54 ` Andres Freund <[email protected]>
2022-07-03 17:07 ` Nathan Bossart <[email protected]>
2022-07-03 17:27 ` Andres Freund <[email protected]>
2022-07-06 16:51 ` Nathan Bossart <[email protected]>
2022-08-11 23:09 ` Nathan Bossart <[email protected]>
2022-08-24 16:46 ` Nathan Bossart <[email protected]>
2024-01-07 22:55 [PATCH v2 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v4 13/19] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v3 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v4 13/19] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v3 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v4 13/19] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v2 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v2 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-01-07 22:55 [PATCH v3 12/17] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-03-26 13:37 [PATCH v7 12/16] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-03-26 13:37 [PATCH v9 12/21] Merge prune and freeze records Melanie Plageman <[email protected]>
2024-03-26 13:37 [PATCH v7 12/16] Merge prune and freeze records 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