public inbox for [email protected]
help / color / mirror / Atom feedMultiple Wait Events for extensions
48+ messages / 9 participants
[nested] [flat]
* Multiple Wait Events for extensions
@ 2018-10-22 17:28 legrand legrand <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: legrand legrand @ 2018-10-22 17:28 UTC (permalink / raw)
To: pgsql-hackers
Hello,
I'm playing with adding into my pg_stat_statements extension a wait event
for pgss time duration (pgss_store)
Adding pgstat_report_wait_start(PG_WAIT_EXTENSION)
gives wait type = "Extension" / event name "Extension"
and that's perfect.
Now I would like to add a second wait event (for exemple a Planner
information based on planner_hook)
Yes I know it's not a "wait", but that's not even possible because
there is only one event type and one event name available for all extensions
...
Could this be changed to offer an extension the ability to log multiple
events
and many extensions to work together ?
What about a pgstat_report_extension_wait_start(i) function
displaying Wait type = "Extension name"
and a predifined event numbers (i in the range 1 to 10) as event name ?
I'm not able to write it, but I'm ready to test it deeply ;o)
Regards
PAscal
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-22 22:47 Michael Paquier <[email protected]>
parent: legrand legrand <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Michael Paquier @ 2018-10-22 22:47 UTC (permalink / raw)
To: legrand legrand <[email protected]>; +Cc: pgsql-hackers
On Mon, Oct 22, 2018 at 10:28:14AM -0700, legrand legrand wrote:
> Could this be changed to offer an extension the ability to log multiple
> events and many extensions to work together?
I recall that this issue has been discussed when adding new wait events,
and we discarded it for simplicity as that's not trivial. A new
facility could be implemented to add custom wait events. This exists
for light-weight locks for example. I am not sure in which shape this
could actually happen though ;)
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-24 18:18 legrand legrand <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: legrand legrand @ 2018-10-24 18:18 UTC (permalink / raw)
To: pgsql-hackers
Would a hard coded solution as described here after possible for mid-term ?
note:
actual result from
pgstat_report_wait_star(PG_WAIT_EXTENSION);
is preserved.
Regards
PAscal
pgstat.h
/* ----------
* Wait Events - Extension
*
* Use this category when an extension is waiting.
* ----------
*/
typedef enum
{
WAIT_EVENT_EXTENSION_EXT = PG_WAIT_EXTENSION,
WAIT_EVENT_EXTENSION_1,
WAIT_EVENT_EXTENSION_2,
WAIT_EVENT_EXTENSION_3,
WAIT_EVENT_EXTENSION_4,
WAIT_EVENT_EXTENSION_5,
WAIT_EVENT_EXTENSION_6,
WAIT_EVENT_EXTENSION_7,
WAIT_EVENT_EXTENSION_8,
WAIT_EVENT_EXTENSION_9,
} WaitEventExtension;
pgstat.c
...
case PG_WAIT_EXTENSION:
{
WaitEventExtension w = (WaitEventExtension) wait_event_info;
event_name = pgstat_get_wait_extension(w);
break;
}
...
/* ----------
* pgstat_get_wait_extension() -
*
* Convert WaitEventExtension to string.
* ----------
*/
static const char *
pgstat_get_wait_extension(WaitEventExtension w)
{
const char *event_name = "unknown wait event";
switch (w)
{
case WAIT_EVENT_EXTENSION_EXT:
event_name = "Extension";
break;
case WAIT_EVENT_EXTENSION_1:
event_name = "Extension_1";
break;
case WAIT_EVENT_EXTENSION_2:
event_name = "Extension_2";
break;
case WAIT_EVENT_EXTENSION_3:
event_name = "Extension_3";
break;
case WAIT_EVENT_EXTENSION_4:
event_name = "Extension_4";
break;
case WAIT_EVENT_EXTENSION_5:
event_name = "Extension_5";
break;
case WAIT_EVENT_EXTENSION_6:
event_name = "Extension_6";
break;
case WAIT_EVENT_EXTENSION_7:
event_name = "Extension_7";
break;
case WAIT_EVENT_EXTENSION_8:
event_name = "Extension_8";
break;
case WAIT_EVENT_EXTENSION_9:
event_name = "Extension_9";
break;
/* no default case, so that compiler will warn */
}
return event_name;
}
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-25 01:14 Michael Paquier <[email protected]>
parent: legrand legrand <[email protected]>
0 siblings, 2 replies; 48+ messages in thread
From: Michael Paquier @ 2018-10-25 01:14 UTC (permalink / raw)
To: legrand legrand <[email protected]>; +Cc: pgsql-hackers
On Wed, Oct 24, 2018 at 11:18:13AM -0700, legrand legrand wrote:
> Would a hard coded solution as described here after possible for
> mid-term?
I don't think I would commit that as we would want a better solution
with custom names, but patching Postgres to do so with a custom build
would be easy enough..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-28 16:57 legrand legrand <[email protected]>
parent: Michael Paquier <[email protected]>
1 sibling, 0 replies; 48+ messages in thread
From: legrand legrand @ 2018-10-28 16:57 UTC (permalink / raw)
To: pgsql-hackers
Michael Paquier-2 wrote
> On Wed, Oct 24, 2018 at 11:18:13AM -0700, legrand legrand wrote:
>> Would a hard coded solution as described here after possible for
>> mid-term?
>
> I don't think I would commit that as we would want a better solution
> with custom names, but patching Postgres to do so with a custom build
> would be easy enough..
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-28 17:12 legrand legrand <[email protected]>
parent: Michael Paquier <[email protected]>
1 sibling, 1 reply; 48+ messages in thread
From: legrand legrand @ 2018-10-28 17:12 UTC (permalink / raw)
To: pgsql-hackers
Michael Paquier-2 wrote
> On Wed, Oct 24, 2018 at 11:18:13AM -0700, legrand legrand wrote:
>> Would a hard coded solution as described here after possible for
>> mid-term?
>
> I don't think I would commit that as we would want a better solution
> with custom names, but patching Postgres to do so with a custom build
> would be easy enough..
I can't deploy a custom build in my context, and will reuse existing spare
events
like
- 0x050E0000U "Activity"-"unknown wait event"
- 0x0B010000U "???"-"unknown wait event"
- ...
An other idea that may be called a "better wait event error handling"
would have be to display:
"???-xx" unknown event type (xx being the associated number)
in pgstat_get_wait_event_type()
"unknown wait event - yy" unknown event name (yy being the associated
number)
in pgstat_get_wait_event()
giving an extended range of spare events ;o)
Regards
PAscal
--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: Multiple Wait Events for extensions
@ 2018-10-29 00:05 Michael Paquier <[email protected]>
parent: legrand legrand <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Michael Paquier @ 2018-10-29 00:05 UTC (permalink / raw)
To: legrand legrand <[email protected]>; +Cc: pgsql-hackers
On Sun, Oct 28, 2018 at 10:12:48AM -0700, legrand legrand wrote:
> An other idea that may be called a "better wait event error handling"
> would have be to display:
>
> "???-xx" unknown event type (xx being the associated number)
> in pgstat_get_wait_event_type()
>
> "unknown wait event - yy" unknown event name (yy being the associated
> number)
> in pgstat_get_wait_event()
>
> giving an extended range of spare events ;o)
Those look like half-baked workarounds in my opinion. What we may want
to finish with is the possibility to register a new event with a custom
string bundled so as an extension loaded via shared_preload_libraries
could call it. That's not a small amount of work.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-09 20:18 Tom Lane <[email protected]>
0 siblings, 4 replies; 48+ messages in thread
From: Tom Lane @ 2023-01-09 20:18 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: vignesh C <[email protected]>; Karl O. Pinc <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
Brar Piening <[email protected]> writes:
> On 09.01.2023 at 03:38, vignesh C wrote:
>> There are couple of commitfest entries for this:
>> https://commitfest.postgresql.org/41/4041/
>> https://commitfest.postgresql.org/41/4042/ Can one of them be closed?
> I've split the initial patch into two parts upon Álvaro's request in [1]
> so that we can discuss them separately
It's not great to have multiple CF entries pointing at the same email
thread --- it confuses both people and bots. Next time please split
off a thread for each distinct patch.
I pushed the ID-addition patch, with a few fixes:
* AFAIK our practice is to use "-" never "_" in XML ID attributes.
You weren't very consistent about that even within this patch, and
the overall effect would have been to have no standard about that
at all, which doesn't seem great. I changed them all to "-".
* I got rid of a couple of "-et-al" additions, because it did not
seem like a good precedent. That would tempt people to modify
existing ID tags when adding variables to an entry, which'd defeat
the purpose I think.
* I fixed a couple of things that looked like typos or unnecessary
inconsistencies. I have to admit that my eyes glazed over after
awhile, so there might be remaining infelicities.
It's probably going to be necessary to have follow-on patches,
because I'm sure there is stuff in the pipeline that adds more
ID-less tags. Or do we have a way to create warnings about that?
I'm unqualified to review CSS stuff, so you'll need to get somebody
else to review that patch. But I'd suggest reposting it, else
the cfbot is going to start whining that the patch-of-record in
this thread no longer applies.
regards, tom lane
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-09 20:45 Karl O. Pinc <[email protected]>
parent: Tom Lane <[email protected]>
3 siblings, 0 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-09 20:45 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Brar Piening <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Mon, 09 Jan 2023 15:18:18 -0500
Tom Lane <[email protected]> wrote:
> I pushed the ID-addition patch, with a few fixes:
>
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great. I changed them all to "-".
Apologies for not catching this.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-09 22:28 Karl O. Pinc <[email protected]>
parent: Tom Lane <[email protected]>
3 siblings, 2 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-09 22:28 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Brar Piening <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Mon, 09 Jan 2023 15:18:18 -0500
Tom Lane <[email protected]> wrote:
> Brar Piening <[email protected]> writes:
> > On 09.01.2023 at 03:38, vignesh C wrote:
> >> There are couple of commitfest entries for this:
> >> https://commitfest.postgresql.org/41/4041/
> >> https://commitfest.postgresql.org/41/4042/ Can one of them be
> >> closed?
>
> > I've split the initial patch into two parts upon Álvaro's request
> > in [1] so that we can discuss them separately
> I pushed the ID-addition patch, with a few fixes:
> It's probably going to be necessary to have follow-on patches,
> because I'm sure there is stuff in the pipeline that adds more
> ID-less tags. Or do we have a way to create warnings about that?
I am unclear on how to make warnings with xslt. You can make
a listing of problems, but who would read it if the build
completed successfully? You can make errors and abort.
But my xslt and docbook and pg-docs-fu are a bit stale.
I think there's more to comment on regards the xslt in the
other patch, but I'll wait for the new thread for that patch.
That might be where there should be warnings raised, etc.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-09 22:30 Tom Lane <[email protected]>
parent: Karl O. Pinc <[email protected]>
1 sibling, 0 replies; 48+ messages in thread
From: Tom Lane @ 2023-01-09 22:30 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Brar Piening <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
"Karl O. Pinc" <[email protected]> writes:
> I think there's more to comment on regards the xslt in the
> other patch, but I'll wait for the new thread for that patch.
> That might be where there should be warnings raised, etc.
We can continue using this thread, now that the other commit is in.
regards, tom lane
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-10 05:12 Brar Piening <[email protected]>
parent: Tom Lane <[email protected]>
3 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-01-10 05:12 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: vignesh C <[email protected]>; Karl O. Pinc <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 09.01.2023 at 21:18, Tom Lane wrote:
> It's not great to have multiple CF entries pointing at the same email
> thread --- it confuses both people and bots. Next time please split
> off a thread for each distinct patch.
I agree. I had overestimated the cfbot's ability to handle branched
threads. I'll create separate threads next time.
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great. I changed them all to "-".
Noted. Maybe it's worth to write a short paragraph about Ids and their
style somewhere in the docs (e.g. Appendix J.5).
> * I got rid of a couple of "-et-al" additions, because it did not
> seem like a good precedent. That would tempt people to modify
> existing ID tags when adding variables to an entry, which'd defeat
> the purpose I think.
I tried to use it sparsely, mostly where a varlistentry had multiple
child items and I had arbitrarily pick one of them. It's not important,
though. I'm curious how you solved this.
> * I fixed a couple of things that looked like typos or unnecessary
> inconsistencies. I have to admit that my eyes glazed over after
> awhile, so there might be remaining infelicities.
I'm all for consistency. The only places where I intentionally refrained
from being consistent was where I felt Ids would get too long or where
there were already ids in place that didn't match my naming scheme.
> It's probably going to be necessary to have follow-on patches,
> because I'm sure there is stuff in the pipeline that adds more
> ID-less tags. Or do we have a way to create warnings about that?
Agreed. And yes, we do have a limited way to create warnings (that's
part of the other patch).
> I'm unqualified to review CSS stuff, so you'll need to get somebody
> else to review that patch. But I'd suggest reposting it, else
> the cfbot is going to start whining that the patch-of-record in
> this thread no longer applies.
I will do that. Thanks for your feedback!
Regards,
Brar
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-10 05:22 Tom Lane <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Tom Lane @ 2023-01-10 05:22 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: vignesh C <[email protected]>; Karl O. Pinc <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
Brar Piening <[email protected]> writes:
> On 09.01.2023 at 21:18, Tom Lane wrote:
>> * I got rid of a couple of "-et-al" additions, because it did not
>> seem like a good precedent. That would tempt people to modify
>> existing ID tags when adding variables to an entry, which'd defeat
>> the purpose I think.
> I tried to use it sparsely, mostly where a varlistentry had multiple
> child items and I had arbitrarily pick one of them. It's not important,
> though. I'm curious how you solved this.
I just removed "-et-al", I didn't question your choice of the principal
variable name. As you say, it didn't seem to matter that much.
regards, tom lane
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-10 05:28 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
1 sibling, 0 replies; 48+ messages in thread
From: Brar Piening @ 2023-01-10 05:28 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; Tom Lane <[email protected]>; +Cc: vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 09.01.2023 at 23:28, Karl O. Pinc wrote:
> On Mon, 09 Jan 2023 15:18:18 -0500
> Tom Lane <[email protected]> wrote:
>> It's probably going to be necessary to have follow-on patches,
>> because I'm sure there is stuff in the pipeline that adds more
>> ID-less tags. Or do we have a way to create warnings about that?
> I am unclear on how to make warnings with xslt. You can make
> a listing of problems, but who would read it if the build
> completed successfully? You can make errors and abort.
You can emit warnings to the command line or you can abort with an
error. I've opted for warnings + comments in the output in the styling
patch.
The biggest issue about errors and warnings is the fact that xslt does
not process files in a line-based way which makes it pretty much
impossible to give hints where the problem causing the warning is
located. Since everything is bound together via XML entities, you can't
even tell the source file.
I've worked around this by also emitting an HTML comment to the output
so that I can find a somewhat unique string next to it and the grep the
documentation sources for this string. It's a bit ugly but the best I
could come up with.
I'll repost a rebased version of the styling patch in a minute.
Regards,
Brar
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-11 14:22 Peter Eisentraut <[email protected]>
parent: Tom Lane <[email protected]>
3 siblings, 1 reply; 48+ messages in thread
From: Peter Eisentraut @ 2023-01-11 14:22 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Brar Piening <[email protected]>; +Cc: vignesh C <[email protected]>; Karl O. Pinc <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 09.01.23 21:18, Tom Lane wrote:
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great. I changed them all to "-".
In the olden says, "_" was invalid in ID attribute values. This is no
longer the case. But of course it's good to stay consistent with
existing practice where reasonable.
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-16 00:01 Karl O. Pinc <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 2 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-16 00:01 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
Hi Brar,
Here's my first review of the make_html_ids_discoverable.patch.
Overall:
To start with, I'd like to say I like the goal and how everything
works when the patch is applied. I was a bit skeptical, thinking that
the whole thing was going to be distracting when reading the docs or
otherwise odd, but it really works.
The thought comes to mind to take accessibility into account. I am
unclear on what that would mean in this case.
Regards CSS:
The CSS looks good. Also, it passes the W3C CSS validator (
https://jigsaw.w3.org/css-validator/) for CSS level 3.
Regards XSLT:
I believe the XSLT needs work.
Without really knowing the style of PG's XSLT, I'm going to suggest
putting the comment
<!-- Make HTML ids discoverable (by hover) by adding links to the ids -->
above the new XSLT. Precede the comment with 2 blank lines and follow
it by one blank line. The idea is to make the code you're adding an
identifiable "separate section".
I have a question, it is the docbook html stylesheets for XML that are
being overridden? It looks like it, and what else would it be, but....
(Sorry, I'm a bit stale on this stuff.)
I believe that overriding the XSLT by copying the original and making
modifications is the "wrong way" (TM). I think that the right way is
to declare a xsl:default-mode somewhere in the stylesheets. There
does not seem to be one, so the default mode for PG doc processing
could be something like "postgres-mode". And I'd expect it to be
declared somewhere right at the top of the xml hierarchy. (I forget
what that is, probably something like "document" or "book" or
something.) Then, you'd write your for the <xsl:template
match="varlistentry"> and <xsl:template name="section.heading"> with a
mode of "postgres-mode", and have your templates call the "traditional
default", "modeless", templates. That way your not copying and
patching upstream code, but are instead, in effect, calling it as a
subroutine.
This should work especially well since, I think, you're just adding
new output to what the upstream templates do. You're not trying to
insert new output into the middle of the stock output or otherwise
modify the stock output.
You're adding only about 6 lines of XSLT to the upstream templates,
and copying 100+ lines. There must be a better way.
See: https://www.w3.org/TR/xslt-30/#modes
I've never tried this, although I do recall doing something or another
with modes in the past. And I've not gone so far as to figure out
(again?) how to call a "modeless template", so you can invoke the
original, upstream, templates. And setting a default mode seems like
something of a "big hammer", so should probably be checked over by
someone who's more steeped in Postgres docs than myself. (Like a
committer. :) But I believe it is the way to go. To make it work
you'll need to figure out the XSLT mode selection process and make
sure that it first selects the "postgres-mode", and then the modeless
templates, and also still works right when a template calls another
and explicitly sets a mode.
Regards visual presentation:
Here's the fun question, what to have "appear" when a section or
varlistentry with an id is hovered over?
I kind of like your choice of the "#" character as the screen element
that becomes visible when you hover over a section or varlistentry
with the mouse, which then shows you the URL of the thing over which
you are hovering. That's what's in the patch now. But I wonder why
you chose it. Is there some sort of standard? I've seen the anchor
Unicode character before, (⚓) \u2693. I don't find it particularly
helpful. The "place of interest" symbol,(⌘) \u2318, might be nice.
There is (◎), \u25ce, the "bullseye" symbol. There is the link symbol
(🔗), \U0001f517. Like the anchor, it has generally been confusing
when I come across it. The link symbol with the "text variant form",
(🔗︎) \U0001f517\ufe0e, looks more link an actual chain and is somewhat
better. (The opposite of "text variant" is "emoji variant".) There
is also the paperclip, (📎) \U0001f4ce. And the paperclip text
variant, (📎︎) \U0001f4ce\ufe0e.
Of all the Unicode choices above, I think I prefer the last. The text
paperclip. 📎︎
(The hex representations above are Python 3 string literals. So
print("\U0001f4ce\ufe0e") prints the text paperclip.)
The actual representation of the various Unicode characters is going
to depend on the font. (Things change for me when looked at in an
email v.s. in a browser.)
The question I have is should we use "Unicode icons" at all or is
plain old UTF-8 good enough for us (because it was good enough for our
ancestors ;) ? Of course an image is also possible.... For now I'm
not going to advocate for a change from the "#" already used in the
patch.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-16 00:28 Karl O. Pinc <[email protected]>
parent: Karl O. Pinc <[email protected]>
1 sibling, 0 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-16 00:28 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Sun, 15 Jan 2023 18:01:50 -0600
"Karl O. Pinc" <[email protected]> wrote:
> Regards XSLT:
>
> I believe the XSLT needs work.
I also think that the XSLT should error and halt
when there's no id (in the expected places).
Instead of just giving a warning and keeping going. Otherwise
they'll constantly be ignored warnings and periodically
there will have to be patches to supply missing ids.
To solve the "which id is missing where so I can fix it"
problem, I propose the error text show the chapter title,
all the enclosing sub-section titles, and any previous existing
varlistentry ids occurring before the tag with the
missing attribute. At least for varlistentry-s. For
sections you could do chapter and enclosing sub-section
titles and the title of the section with the problem.
That should be enough for an author to find the place
in the source sgml that needs fixing.
Maybe, possibly, you can see how this is done by looking
at whatever XSLT there is that automatically generates
ids for sections without ids, so that the table of contents
have something to link to. In any case, XSLT is really
good at "looking at" parent/enclosing XML, so producing
a useful error message shouldn't be _that_ hard. I've
definitely done this sort of thing before so I can tell you
it's readily doable.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-16 17:14 Karl O. Pinc <[email protected]>
parent: Karl O. Pinc <[email protected]>
1 sibling, 1 reply; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-16 17:14 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Sun, 15 Jan 2023 18:01:50 -0600
"Karl O. Pinc" <[email protected]> wrote:
> Regards XSLT:
>
> I believe the XSLT needs work.
> I believe that overriding the XSLT by copying the original and making
> modifications is the "wrong way" (TM). I think that the right way is
> to declare a xsl:default-mode somewhere in the stylesheets. There
> does not seem to be one, so the default mode for PG doc processing
> could be something like "postgres-mode". And I'd expect it to be
> declared somewhere right at the top of the xml hierarchy. (I forget
> what that is, probably something like "document" or "book" or
> something.) Then, you'd write your for the <xsl:template
> match="varlistentry"> and <xsl:template name="section.heading"> with
> a mode of "postgres-mode", and have your templates call the
> "traditional default", "modeless", templates. That way your not
> copying and patching upstream code, but are instead, in effect,
> calling it as a subroutine.
>
> This should work especially well since, I think, you're just adding
> new output to what the upstream templates do. You're not trying to
> insert new output into the middle of the stock output or otherwise
> modify the stock output.
>
> You're adding only about 6 lines of XSLT to the upstream templates,
> and copying 100+ lines. There must be a better way.
>
> See: https://www.w3.org/TR/xslt-30/#modes
>
> I've never tried this, although I do recall doing something or another
> with modes in the past. And I've not gone so far as to figure out
> (again?) how to call a "modeless template", so you can invoke the
> original, upstream, templates. And setting a default mode seems like
> something of a "big hammer", so should probably be checked over by
> someone who's more steeped in Postgres docs than myself. (Like a
> committer. :) But I believe it is the way to go. To make it work
> you'll need to figure out the XSLT mode selection process and make
> sure that it first selects the "postgres-mode", and then the modeless
> templates, and also still works right when a template calls another
> and explicitly sets a mode.
Drat. I forgot. We're using xsltproc which is XSLT 1.0.
So this is the relevant spec:
https://www.w3.org/TR/1999/REC-xslt-19991116#modes
In XSLT 1.0 there is no xml:default-mode. So I _think_ what you do then
is modify the built-in template rules so that the (default) template
(mode='') is invoked when there is no 'postgres-mode' version of the
template, but otherwise the 'postgres-mode' version of the template
is invoked. Your 'postgres-mode' templates will xsl:call-template
the default template, adding whatever they want to the output produced
by the default template.
See: https://www.w3.org/TR/1999/REC-xslt-19991116#built-in-rule
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-17 01:05 Karl O. Pinc <[email protected]>
parent: Karl O. Pinc <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-17 01:05 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Mon, 16 Jan 2023 11:14:35 -0600
"Karl O. Pinc" <[email protected]> wrote:
> On Sun, 15 Jan 2023 18:01:50 -0600
> "Karl O. Pinc" <[email protected]> wrote:
>
> > Regards XSLT:
> >
> > I believe the XSLT needs work.
> In XSLT 1.0 there is no xml:default-mode. So I _think_ what you do
> then is modify the built-in template rules so that the (default)
> template (mode='') is invoked when there is no 'postgres-mode'
> version of the template, but otherwise the 'postgres-mode' version of
> the template is invoked. Your 'postgres-mode' templates will
> xsl:call-template the default template, adding whatever they want to
> the output produced by the default template.
Or maybe the right way is to set a mode at the very top,
the first apply-templates call, and not mess with the
built-in templates at all. (You'd write your own
"postgres-mode" templates the same way, to "wrap"
and call the default templates.)
Think of the mode as an implicit argument that's preserved and
passed down through each template invocation without having to
be explicitly specified by the calling code.
Regards
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-17 05:57 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-01-17 05:57 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 17.01.2023 at 02:05, Karl O. Pinc wrote:
> Or maybe the right way is to set a mode at the very top,
> the first apply-templates call, and not mess with the
> built-in templates at all. (You'd write your own
> "postgres-mode" templates the same way, to "wrap"
> and call the default templates.)
>
> Think of the mode as an implicit argument that's preserved and
> passed down through each template invocation without having to
> be explicitly specified by the calling code.
I think the document you're missing is [1].
There are multiple ways to customize DocBook XSL output and it sounds
like you want me to write a customization layer which I didn't do
because there is precedent that the typical "way to do it" (TM) in the
PostgreSQL project is [2].
Regards,
Brar
[1] http://www.sagehill.net/docbookxsl/CustomizingPart.html
[2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-17 13:12 Karl O. Pinc <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-17 13:12 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Tue, 17 Jan 2023 06:57:23 +0100
Brar Piening <[email protected]> wrote:
> On 17.01.2023 at 02:05, Karl O. Pinc wrote:
> > Or maybe the right way is to set a mode at the very top,
> > the first apply-templates call, and not mess with the
> > built-in templates at all. (You'd write your own
> > "postgres-mode" templates the same way, to "wrap"
> > and call the default templates.)
> >
> > Think of the mode as an implicit argument that's preserved and
> > passed down through each template invocation without having to
> > be explicitly specified by the calling code.
>
> I think the document you're missing is [1].
>
> There are multiple ways to customize DocBook XSL output and it sounds
> like you want me to write a customization layer which I didn't do
> because there is precedent that the typical "way to do it" (TM) in the
> PostgreSQL project is [2].
>
> Regards,
>
> Brar
>
> [1] http://www.sagehill.net/docbookxsl/CustomizingPart.html
> [2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html
>
Sagehill is normally vary good. But in this case [2] does not
apply. Or rather it applies but it is overkill because you
do not want to replace what a template is producing. You
want to add to what a template is producing. So you want to
wrap the template, with your new code adding output before/
after what the original produces.
[1] does not contain this technique.
If you're not willing to try I am willing to see if I can
produce an example to work from. My XSLT is starting to
come back.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-17 18:13 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-01-17 18:13 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 17.01.2023 at 14:12, Karl O. Pinc wrote:
> If you're not willing to try I am willing to see if I can
> produce an example to work from. My XSLT is starting to
> come back.
I'm certainly willing to try but I'd appreciate an example in any case.
My XSLT skills are mostly learning by doing combined with trial and error.
Regards,
Brar
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-17 22:43 Karl O. Pinc <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 3 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-17 22:43 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Tue, 17 Jan 2023 19:13:38 +0100
Brar Piening <[email protected]> wrote:
> On 17.01.2023 at 14:12, Karl O. Pinc wrote:
> > If you're not willing to try I am willing to see if I can
> > produce an example to work from. My XSLT is starting to
> > come back.
>
> I'm certainly willing to try but I'd appreciate an example in any
> case.
It's good you asked. After poking about the XSLT 1.0 spec to refresh
my memory I abandoned the "mode" approach entirely. The real "right
way" is to use the import mechanism.
I've attached a patch that "wraps" the section.heading template
and adds extra stuff to the HTML output generated by the stock
template. (example_section_heading_override.patch)
There's a bug. All that goes into the html is a comment, not
a hoverable link. But the technique is clear.
On my system (Debian 11, bullseye) I found the URI to import
by looking at:
/usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml
(Which is probably the right place to look.)
Ultimately, that file is findable via: /etc/xml/catalog
The "best way" on
Debian is: /usr/share/doc/docbook-xsl/README.gz
In other words, the README that comes with the style sheets.
Supposedly, the href=URLs are really URIs and will be good
no matter what/when. The XSLT processor should know to look
at the system catalogs and read the imported style sheet
from the local file system.
It might be useful to add --nonet to the xsltproc invocation(s)
in the Makefile(s). Just in case; to keep from retrieving
stylesheets from the net. (If the option is not already there.
I didn't look.)
If this is the first time that PG uses the XSLT import mechanism
I imagine that "things could go wrong" depending on what sort
of system is being used to build the docs. I'm not worried,
but it is something to note for the committer.
> My XSLT skills are mostly learning by doing combined with trial and
> error.
I think of XSLT as a functional programming language. Recursion is
a big deal, and data directed programming can be a powerful technique
because XSLT is so good with data structures.
(https://mitp-content-server.mit.edu/books/content/sectbyfn/books_pres_0/6515/sicp.zip/full-text/book...)
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Attachments:
[text/x-patch] example_section_heading_overrride.patch (3.0K, ../../[email protected]/2-example_section_heading_overrride.patch)
download | inline diff:
diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl
index 9df2782ce4..88e084e190 100644
--- a/doc/src/sgml/stylesheet-html-common.xsl
+++ b/doc/src/sgml/stylesheet-html-common.xsl
@@ -12,6 +12,10 @@
all HTML output variants (chunked and single-page).
-->
+<!-- Imports - to override stylesheet templates -->
+<xsl:import
+ href="http://cdn.docbook.org/release/xsl/current/html/sections.xsl"/>
+
<!-- Parameters -->
<xsl:param name="make.valid.html" select="1"></xsl:param>
<xsl:param name="generate.id.attributes" select="1"></xsl:param>
@@ -301,4 +305,53 @@ set toc,title
</xsl:choose>
</xsl:template>
+<!-- Override the standard section heading generation to add an id link -->
+<xsl:template name="section.heading">
+ <xsl:param name="section" select="."/>
+ <xsl:param name="level" select="1"/>
+ <xsl:param name="allow-anchors" select="1"/>
+ <xsl:param name="title"/>
+ <xsl:param name="class" select="'title'"/>
+ <xsl:apply-imports/>
+ <xsl:call-template name="pg.id.link">
+ <xsl:with-param name="object" select="$section"/>
+ </xsl:call-template>
+</xsl:template>
+
+<!-- Creates a link pointing to an id within the document -->
+<xsl:template name="pg.id.link">
+ <xsl:param name="object" select="."/>
+ <xsl:choose>
+ <xsl:when test="$object/@id or $object/@xml:id">
+ <a>
+ <xsl:attribute name="href">
+ <xsl:text>#</xsl:text>
+ <xsl:call-template name="object.id">
+ <xsl:with-param name="object" select="$object"/>
+ </xsl:call-template>
+ </xsl:attribute>
+ <xsl:attribute name="class">
+ <xsl:text>id_link</xsl:text>
+ </xsl:attribute>
+ <xsl:text> #</xsl:text>
+ </a>
+ </xsl:when>
+ <xsl:otherwise>
+ <!-- Only complain about varlistentries if at least one entry in the list has an id -->
+ <xsl:if test="name($object) != 'varlistentry' or $object/parent::variablelist/varlistentry[@id]">
+ <xsl:message terminate="no">
+ <xsl:value-of select ="name($object)"/>
+ <xsl:text> element without id. Please add an id to make it usable</xsl:text>
+ <xsl:text> as stable anchor in the public HTML documentation.</xsl:text>
+ </xsl:message>
+ <xsl:comment>
+ <xsl:text>Please add an id to the </xsl:text>
+ <xsl:value-of select ="name($object)"/>
+ <xsl:text> element in the sgml source code.</xsl:text>
+ </xsl:comment>
+ </xsl:if>
+ </xsl:otherwise>
+ </xsl:choose>
+</xsl:template>
+
</xsl:stylesheet>
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index 6410a47958..e4174e0613 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -163,3 +163,13 @@ acronym { font-style: inherit; }
width: 75%;
}
}
+
+/* Links to ids of headers and definition terms */
+a.id_link {
+ color: inherit;
+ visibility: hidden;
+}
+
+*:hover > a.id_link {
+ visibility: visible;
+}
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-18 05:50 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
2 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-01-18 05:50 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> It's good you asked. After poking about the XSLT 1.0 spec to refresh
> my memory I abandoned the "mode" approach entirely. The real "right
> way" is to use the import mechanism.
>
> I've attached a patch that "wraps" the section.heading template
> and adds extra stuff to the HTML output generated by the stock
> template. (example_section_heading_override.patch)
Thanks!
I'll give it a proper look this weekend.
Regards,
Brar
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-19 04:05 Karl O. Pinc <[email protected]>
parent: Karl O. Pinc <[email protected]>
2 siblings, 0 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-01-19 04:05 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Tue, 17 Jan 2023 16:43:13 -0600
"Karl O. Pinc" <[email protected]> wrote:
> It might be useful to add --nonet to the xsltproc invocation(s)
> in the Makefile(s). Just in case; to keep from retrieving
> stylesheets from the net. (If the option is not already there.
> I didn't look.)
>
> If this is the first time that PG uses the XSLT import mechanism
> I imagine that "things could go wrong" depending on what sort
> of system is being used to build the docs. I'm not worried,
> but it is something to note for the committer.
Looks like doc/src/sgml/stylesheet-fo.xsl already uses
xsl:import, although it is unclear to me whether the import
is applied.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-01-26 20:48 Brar Piening <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 2 replies; 48+ messages in thread
From: Brar Piening @ 2023-01-26 20:48 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 18.01.2023 at 06:50, Brar Piening wrote:
> I'll give it a proper look this weekend.
It turns out I didn't get a round tuit.
... and I'm afraid I probably will not be able to work on this until
mid/end February so we'll have to move this to the next commitfest until
somebody wants to take it over and push it through.
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-02-14 20:13 Andres Freund <[email protected]>
parent: Brar Piening <[email protected]>
1 sibling, 1 reply; 48+ messages in thread
From: Andres Freund @ 2023-02-14 20:13 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
Hi,
On 2023-01-26 21:48:54 +0100, Brar Piening wrote:
> On 18.01.2023 at 06:50, Brar Piening wrote:
>
> > I'll give it a proper look this weekend.
>
> It turns out I didn't get a round tuit.
>
> ... and I'm afraid I probably will not be able to work on this until
> mid/end February so we'll have to move this to the next commitfest until
> somebody wants to take it over and push it through.
A small note: As-is this fails on CI, because we don't allow network access
during the docs build anymore (since it always fails these days):
https://cirrus-ci.com/task/5474029402849280?logs=docs_build#L297
[17:02:03.114] time make -s -j${BUILD_JOBS} -C doc
[17:02:04.092] I/O error : Attempt to load network entity http://cdn.docbook.org/release/xsl/current/html/sections.xsl
[17:02:04.092] warning: failed to load external entity "http://cdn.docbook.org/release/xsl/current/html/sections.xsl";
[17:02:04.092] compilation error: file stylesheet-html-common.xsl line 17 element import
[17:02:04.092] xsl:import : unable to load http://cdn.docbook.org/release/xsl/current/html/sections.xsl
I think this is just due to the common URL in docbook packages being
http://docbook.sourceforge.net/release/xsl/current/*
Because of that the docbook catalog matching logic won't work for that file:
E.g. I have the following in /etc/xml/docbook-xsl.xml, on debian unstable:
<delegateURI uriStartString="http://docbook.sourceforge.net/release/xsl/"; catalog="file:///usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml"/>
As all our other references use the sourceforge address, this should too.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-02-15 19:34 Karl O. Pinc <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Karl O. Pinc @ 2023-02-15 19:34 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Brar Piening <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Tue, 14 Feb 2023 12:13:18 -0800
Andres Freund <[email protected]> wrote:
> A small note: As-is this fails on CI, because we don't allow network
> access during the docs build anymore (since it always fails these
> days):
>
> https://cirrus-ci.com/task/5474029402849280?logs=docs_build#L297
>
> [17:02:03.114] time make -s -j${BUILD_JOBS} -C doc
> [17:02:04.092] I/O error : Attempt to load network entity
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl
> [17:02:04.092] warning: failed to load external entity
> "http://cdn.docbook.org/release/xsl/current/html/sections.xsl";
> [17:02:04.092] compilation error: file stylesheet-html-common.xsl
> line 17 element import [17:02:04.092] xsl:import : unable to load
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl
This makes me think that it would be useful to add --nonet to the
xsltproc invocations. That would catch this error before it goes to
CI.
> I think this is just due to the common URL in docbook packages being
> http://docbook.sourceforge.net/release/xsl/current/*
> Because of that the docbook catalog matching logic won't work for
> that file:
>
> E.g. I have the following in /etc/xml/docbook-xsl.xml, on debian
> unstable: <delegateURI
> uriStartString="http://docbook.sourceforge.net/release/xsl/";
> catalog="file:///usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml"/>
>
> As all our other references use the sourceforge address, this should
> too.
Agreed.
I'm also noticing that the existing xsl:import-s all import entire
docbook stylesheets. It does not hurt to do this; the output is
unaffected, although I can't say what it means for build performance.
It does keep it simple. Only one import is needed no matter which
templates we use the import mechanism to extend. And by importing
"everything" there's no concern about any (unlikely) changes to
the the "internals" of the catalog.
Should we import only what we need or all of docbook? I don't know.
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-02-15 20:49 Andres Freund <[email protected]>
parent: Karl O. Pinc <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Andres Freund @ 2023-02-15 20:49 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Brar Piening <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
Hi,
On 2023-02-15 13:34:37 -0600, Karl O. Pinc wrote:
> This makes me think that it would be useful to add --nonet to the
> xsltproc invocations. That would catch this error before it goes to
> CI.
We are doing that now :)
commit 969509c3f2e3b4c32dcf264f9d642b5ef01319f3
Author: Tom Lane <[email protected]>
Date: 2023-02-08 17:15:23 -0500
Stop recommending auto-download of DTD files, and indeed disable it.
> I'm also noticing that the existing xsl:import-s all import entire
> docbook stylesheets. It does not hurt to do this; the output is
> unaffected, although I can't say what it means for build performance.
> It does keep it simple. Only one import is needed no matter which
> templates we use the import mechanism to extend. And by importing
> "everything" there's no concern about any (unlikely) changes to
> the the "internals" of the catalog.
>
> Should we import only what we need or all of docbook? I don't know.
It couldn't hurt to check if performance improves when you avoid doing so. I
suspect it won't make much of a difference, because the time is actually spent
evaluating xslt rather than parsing it.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-20 18:47 Gregory Stark (as CFM) <[email protected]>
parent: Brar Piening <[email protected]>
1 sibling, 1 reply; 48+ messages in thread
From: Gregory Stark (as CFM) @ 2023-03-20 18:47 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Thu, 26 Jan 2023 at 15:55, Brar Piening <[email protected]> wrote:
>
> On 18.01.2023 at 06:50, Brar Piening wrote:
>
> > I'll give it a proper look this weekend.
>
> It turns out I didn't get a round tuit.
>
> ... and I'm afraid I probably will not be able to work on this until
> mid/end February so we'll have to move this to the next commitfest until
> somebody wants to take it over and push it through.
Looks like a lot of good work was happening on this patch right up
until mid-February. Is there a lot of work left? Do you think you'll
have a chance to wrap it up this commitfest for this release?
--
Gregory Stark
As Commitfest Manager
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-21 13:57 Brar Piening <[email protected]>
parent: Gregory Stark (as CFM) <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Brar Piening @ 2023-03-21 13:57 UTC (permalink / raw)
To: Gregory Stark (as CFM) <[email protected]>; +Cc: Karl O. Pinc <[email protected]>; Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 20.03.2023 at 19:47, Gregory Stark (as CFM) wrote:
> Looks like a lot of good work was happening on this patch right up
> until mid-February. Is there a lot of work left? Do you think you'll
> have a chance to wrap it up this commitfest for this release?
Thanks for the ping.
I had another look this morning and I think I can probably finish this
by the end of the week.
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-21 22:16 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
2 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-03-21 22:16 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> It's good you asked. After poking about the XSLT 1.0 spec to refresh
> my memory I abandoned the "mode" approach entirely. The real "right
> way" is to use the import mechanism.
It actually is not.
After quite some time trying to figure out why things don't work as
intended, I ended up reading the XSLT 1.0 spec.
As the name already suggests, <xsl:apply-imports> is closely related to
<xsl:apply-templates> with the difference that it *applies* a *template
rule* from an imported style sheet instead of applying a template rule
from the current style sheet
(https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it
does not do is *calling* a *named template*
(https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates).
What this basically means is that in XSLT 1.0 you can use
<xsl:apply-imports> to override template rules (<xsl:template
match="this-pattern-inside-match-makes-it-a-template-rule">) but you
cannot use it to override named templates (<xsl:template
name="this-id-inside-name-makes-it-a-named-template">). If you want to
override named templates you basically have to duplicate and change them.
While there are mechanisms to call overriden named templates in XSLT 3,
this is out of scope here, since we're bound to XSLT 1.0
As a consequence, there was little I could change in the initial patch
to avoid the code duplication and all attempts to do so, ultimately led
to even longer and more complex code without really reducing the amount
of duplication.
The <xsl:apply-imports> approach actually does work in the varlistentry
case, although this doesn't really change a lot regarding the length of
the patch (it's a bit nicer though since in this case it really avoids
duplication). I've also taken the advice to terminate the build and
print the xpath if a required id is missing.
The attached patch is my best-effort approach to implement discoverable
links.
Best regards,
Brar
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 62d9f9eb22..9a0241a8d6 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795
</listitem>
</varlistentry>
- <varlistentry id="pgwalinspect-funcs-pg-get-wal-block-info">
+ <varlistentry>
<term>
<function>pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
</term>
diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl
index 3f85ea9536..9df2782ce4 100644
--- a/doc/src/sgml/stylesheet-html-common.xsl
+++ b/doc/src/sgml/stylesheet-html-common.xsl
@@ -301,120 +301,4 @@ set toc,title
</xsl:choose>
</xsl:template>
-<!-- Override the standard section heading generation to add an id link -->
-<xsl:template name="section.heading">
- <xsl:param name="section" select="."/>
- <xsl:param name="level" select="1"/>
- <xsl:param name="allow-anchors" select="1"/>
- <xsl:param name="title"/>
- <xsl:param name="class" select="'title'"/>
-
- <xsl:variable name="id">
- <xsl:choose>
- <!-- Make sure the subtitle doesn't get the same id as the title -->
- <xsl:when test="self::subtitle">
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="."/>
- </xsl:call-template>
- </xsl:when>
- <!-- if title is in an *info wrapper, get the grandparent -->
- <xsl:when test="contains(local-name(..), 'info')">
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="../.."/>
- </xsl:call-template>
- </xsl:when>
- <xsl:otherwise>
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select=".."/>
- </xsl:call-template>
- </xsl:otherwise>
- </xsl:choose>
- </xsl:variable>
-
- <!-- HTML H level is one higher than section level -->
- <xsl:variable name="hlevel">
- <xsl:choose>
- <!-- highest valid HTML H level is H6; so anything nested deeper
- than 5 levels down just becomes H6 -->
- <xsl:when test="$level > 5">6</xsl:when>
- <xsl:otherwise>
- <xsl:value-of select="$level + 1"/>
- </xsl:otherwise>
- </xsl:choose>
- </xsl:variable>
- <xsl:element name="h{$hlevel}" namespace="http://www.w3.org/1999/xhtml";
- <xsl:attribute name="class"><xsl:value-of select="$class"/></xsl:attribute>
- <xsl:if test="$css.decoration != '0'">
- <xsl:if test="$hlevel<3">
- <xsl:attribute name="style">clear: both</xsl:attribute>
- </xsl:if>
- </xsl:if>
- <xsl:if test="$allow-anchors != 0">
- <xsl:call-template name="anchor">
- <xsl:with-param name="node" select="$section"/>
- <xsl:with-param name="conditional" select="0"/>
- </xsl:call-template>
- </xsl:if>
- <xsl:copy-of select="$title"/>
- <xsl:call-template name="pg.id.link">
- <xsl:with-param name="object" select="$section"/>
- </xsl:call-template>
- </xsl:element>
-</xsl:template>
-
-<!-- Override the standard varlistentry/term generation to add an id link
- after the last term -->
-<xsl:template match="varlistentry/term">
- <xsl:apply-imports/>
- <xsl:if test="position() = last()">
- <xsl:call-template name="pg.id.link">
- <xsl:with-param name="object" select="parent::varlistentry"/>
- </xsl:call-template>
- </xsl:if>
-</xsl:template>
-
-<!-- Creates a link pointing to an id within the document -->
-<xsl:template name="pg.id.link">
- <xsl:param name="object" select="."/>
- <xsl:choose>
- <xsl:when test="$object/@id or $object/@xml:id">
- <a>
- <xsl:attribute name="href">
- <xsl:text>#</xsl:text>
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="$object"/>
- </xsl:call-template>
- </xsl:attribute>
- <xsl:attribute name="class">
- <xsl:text>id_link</xsl:text>
- </xsl:attribute>
- <xsl:text> #</xsl:text>
- </a>
- </xsl:when>
- <xsl:otherwise>
- <!-- Only complain about varlistentries if at least one entry in the list has an id -->
- <xsl:if test="name($object) != 'varlistentry' or $object/parent::variablelist/varlistentry[@id]">
- <xsl:message terminate="yes">
- <xsl:text>A </xsl:text>
- <xsl:value-of select ="name($object)"/>
- <xsl:text> element at path '</xsl:text>
- <xsl:for-each select="$object/ancestor::*">
- <xsl:text>/</xsl:text>
- <xsl:value-of select ="name(.)"/>
- <xsl:if test="@id|@xml:id">
- <xsl:text>[@</xsl:text>
- <xsl:value-of select ="name(@id|@xml:id)"/>
- <xsl:text> = '</xsl:text>
- <xsl:value-of select ="@id"/>
- <xsl:text>']</xsl:text>
- </xsl:if>
- </xsl:for-each>
- <xsl:text>' is missing an id. Please add one to make it usable</xsl:text>
- <xsl:text> as stable anchor in the public HTML documentation.</xsl:text>
- </xsl:message>
- </xsl:if>
- </xsl:otherwise>
- </xsl:choose>
-</xsl:template>
-
</xsl:stylesheet>
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index 15bcc95d41..cc14efa1ca 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -169,13 +169,3 @@ acronym { font-style: inherit; }
width: 75%;
}
}
-
-/* Links to ids of headers and definition terms */
-a.id_link {
- color: inherit;
- visibility: hidden;
-}
-
-*:hover > a.id_link {
- visibility: visible;
-}
Attachments:
[text/plain] make_html_ids_discoverable_v3.patch (5.6K, ../../[email protected]/2-make_html_ids_discoverable_v3.patch)
download | inline diff:
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 62d9f9eb22..9a0241a8d6 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795
</listitem>
</varlistentry>
- <varlistentry id="pgwalinspect-funcs-pg-get-wal-block-info">
+ <varlistentry>
<term>
<function>pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
</term>
diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl
index 3f85ea9536..9df2782ce4 100644
--- a/doc/src/sgml/stylesheet-html-common.xsl
+++ b/doc/src/sgml/stylesheet-html-common.xsl
@@ -301,120 +301,4 @@ set toc,title
</xsl:choose>
</xsl:template>
-<!-- Override the standard section heading generation to add an id link -->
-<xsl:template name="section.heading">
- <xsl:param name="section" select="."/>
- <xsl:param name="level" select="1"/>
- <xsl:param name="allow-anchors" select="1"/>
- <xsl:param name="title"/>
- <xsl:param name="class" select="'title'"/>
-
- <xsl:variable name="id">
- <xsl:choose>
- <!-- Make sure the subtitle doesn't get the same id as the title -->
- <xsl:when test="self::subtitle">
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="."/>
- </xsl:call-template>
- </xsl:when>
- <!-- if title is in an *info wrapper, get the grandparent -->
- <xsl:when test="contains(local-name(..), 'info')">
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="../.."/>
- </xsl:call-template>
- </xsl:when>
- <xsl:otherwise>
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select=".."/>
- </xsl:call-template>
- </xsl:otherwise>
- </xsl:choose>
- </xsl:variable>
-
- <!-- HTML H level is one higher than section level -->
- <xsl:variable name="hlevel">
- <xsl:choose>
- <!-- highest valid HTML H level is H6; so anything nested deeper
- than 5 levels down just becomes H6 -->
- <xsl:when test="$level > 5">6</xsl:when>
- <xsl:otherwise>
- <xsl:value-of select="$level + 1"/>
- </xsl:otherwise>
- </xsl:choose>
- </xsl:variable>
- <xsl:element name="h{$hlevel}" namespace="http://www.w3.org/1999/xhtml">
- <xsl:attribute name="class"><xsl:value-of select="$class"/></xsl:attribute>
- <xsl:if test="$css.decoration != '0'">
- <xsl:if test="$hlevel<3">
- <xsl:attribute name="style">clear: both</xsl:attribute>
- </xsl:if>
- </xsl:if>
- <xsl:if test="$allow-anchors != 0">
- <xsl:call-template name="anchor">
- <xsl:with-param name="node" select="$section"/>
- <xsl:with-param name="conditional" select="0"/>
- </xsl:call-template>
- </xsl:if>
- <xsl:copy-of select="$title"/>
- <xsl:call-template name="pg.id.link">
- <xsl:with-param name="object" select="$section"/>
- </xsl:call-template>
- </xsl:element>
-</xsl:template>
-
-<!-- Override the standard varlistentry/term generation to add an id link
- after the last term -->
-<xsl:template match="varlistentry/term">
- <xsl:apply-imports/>
- <xsl:if test="position() = last()">
- <xsl:call-template name="pg.id.link">
- <xsl:with-param name="object" select="parent::varlistentry"/>
- </xsl:call-template>
- </xsl:if>
-</xsl:template>
-
-<!-- Creates a link pointing to an id within the document -->
-<xsl:template name="pg.id.link">
- <xsl:param name="object" select="."/>
- <xsl:choose>
- <xsl:when test="$object/@id or $object/@xml:id">
- <a>
- <xsl:attribute name="href">
- <xsl:text>#</xsl:text>
- <xsl:call-template name="object.id">
- <xsl:with-param name="object" select="$object"/>
- </xsl:call-template>
- </xsl:attribute>
- <xsl:attribute name="class">
- <xsl:text>id_link</xsl:text>
- </xsl:attribute>
- <xsl:text> #</xsl:text>
- </a>
- </xsl:when>
- <xsl:otherwise>
- <!-- Only complain about varlistentries if at least one entry in the list has an id -->
- <xsl:if test="name($object) != 'varlistentry' or $object/parent::variablelist/varlistentry[@id]">
- <xsl:message terminate="yes">
- <xsl:text>A </xsl:text>
- <xsl:value-of select ="name($object)"/>
- <xsl:text> element at path '</xsl:text>
- <xsl:for-each select="$object/ancestor::*">
- <xsl:text>/</xsl:text>
- <xsl:value-of select ="name(.)"/>
- <xsl:if test="@id|@xml:id">
- <xsl:text>[@</xsl:text>
- <xsl:value-of select ="name(@id|@xml:id)"/>
- <xsl:text> = '</xsl:text>
- <xsl:value-of select ="@id"/>
- <xsl:text>']</xsl:text>
- </xsl:if>
- </xsl:for-each>
- <xsl:text>' is missing an id. Please add one to make it usable</xsl:text>
- <xsl:text> as stable anchor in the public HTML documentation.</xsl:text>
- </xsl:message>
- </xsl:if>
- </xsl:otherwise>
- </xsl:choose>
-</xsl:template>
-
</xsl:stylesheet>
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index 15bcc95d41..cc14efa1ca 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -169,13 +169,3 @@ acronym { font-style: inherit; }
width: 75%;
}
}
-
-/* Links to ids of headers and definition terms */
-a.id_link {
- color: inherit;
- visibility: hidden;
-}
-
-*:hover > a.id_link {
- visibility: visible;
-}
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-23 03:09 Karl O. Pinc <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Karl O. Pinc @ 2023-03-23 03:09 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Tue, 21 Mar 2023 23:16:25 +0100
Brar Piening <[email protected]> wrote:
> On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> > It's good you asked. After poking about the XSLT 1.0 spec to
> > refresh my memory I abandoned the "mode" approach entirely. The
> > real "right way" is to use the import mechanism.
> After quite some time trying to figure out why things don't work as
> intended, I ended up reading the XSLT 1.0 spec.
>
> As the name already suggests, <xsl:apply-imports> is closely related
> to <xsl:apply-templates> with the difference that it *applies* a
> *template rule* from an imported style sheet instead of applying a
> template rule from the current style sheet
> (https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it
> does not do is *calling* a *named template*
> (https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates).
>
> What this basically means is that in XSLT 1.0 you can use
> <xsl:apply-imports> to override template rules (<xsl:template
> match="this-pattern-inside-match-makes-it-a-template-rule">) but you
> cannot use it to override named templates (<xsl:template
> name="this-id-inside-name-makes-it-a-named-template">). If you want to
> override named templates you basically have to duplicate and change
> them.
>
> While there are mechanisms to call overriden named templates in XSLT
> 3, this is out of scope here, since we're bound to XSLT 1.0
(It was reassuring to see you take the steps above; I once did exactly
the same with and had the same excitements and disappointments. I
feel validated. ;-)
(One of my disappointments is that xsltproc supports only XSLT 1.0,
and nothing later. IIRC, apparently one big reason is not the amount
work needed to develop the program but the work required to develop a
test suite to validate conformance.)
> As a consequence, there was little I could change in the initial patch
> to avoid the code duplication and all attempts to do so, ultimately
> led to even longer and more complex code without really reducing the
> amount of duplication.
You're quite right. I clearly didn't have my XSLT turned on. Importing
only works when templates are matched, not called by name.
Sorry for the extra work I've put you through.
> The <xsl:apply-imports> approach actually does work in the
> varlistentry case, although this doesn't really change a lot
> regarding the length of the patch (it's a bit nicer though since in
> this case it really avoids duplication).
You've put in a lot of good work. I'm attaching 2 patches
with only minor changes.
001-add-needed-ids_v1.patch
This separates out the addition of ids from the XSLT changes, just
to keep things tidy. Content is from your patch.
002-make_html_ids_discoverable_v4.patch
I changed the linked text, the #, so that the leading space
is not linked. This is arguable, as the extra space makes
it easier to put the mouse on the region. But it seems
tidy.
I've tided up so the lines are no longer than 80 chars.
> I've also taken the advice
> to terminate the build and print the xpath if a required id is
> missing.
This looks awesome. I love the xpath! I've changed the format of the
error message. What do you think? (Try it out by _not_ applying
001-add-needed-ids_v1.patch.)
Also, the error message now has leading and trailing newlines to make
it stand out. I'm normally against this sort of thing but thought I'd
add it anyway for others to review.
I'm ready to send these on to a committer but if you don't
like what I did please send more patches for me to review.
Outstanding questions (for committer?):
The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
<h2>, etc. attributes using a XSLT <element> element with a
"namespace" attribute. I'm unclear on the relationship PG has with
xhtml and namespaces. Looks right to me, since the generated html has
the same namespace name appearing in the xmlns attribute of the html
tag, but somebody who knows more than me might want to review this.
Using the namespace attribute does not seem to have affected the
generated html, as far as my random sampling of output can tell.
What character should be used to represent a link anchor?
I've left your choice of "#" in the patch.
If we go to unicode, My preference is the text paperclip 📎︎
Here's a table of all the choices I came up with, there may be others
that are suitable. (The hex representations are Python 3 string
literals. So print("\U0001f4ce\ufe0e") prints the text paperclip.)
Hash mark # (ASCII, used in the patch, \u0023)
Anchor ⚓ \u2693
Place of interest ⌘ \u2318
Bullseye ◎ \u25ce
Link (emoji variant) 🔗 \U0001f517
Link (text variant) 🔗︎ \U0001f517\ufe0e
Paperclip (emoji variant) 📎 \U0001f4ce
Paperclip (text variant) 📎︎ \U0001f4ce\ufe0e
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Attachments:
[text/x-patch] 001-add-needed-ids_v1.patch (509B, ../../[email protected]/2-001-add-needed-ids_v1.patch)
download | inline diff:
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 9a0241a8d6..62d9f9eb22 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -157,7 +157,7 @@ combined_size_percentage | 2.8634072910530795
</listitem>
</varlistentry>
- <varlistentry>
+ <varlistentry id="pgwalinspect-funcs-pg-get-wal-block-info">
<term>
<function>pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
</term>
[text/x-patch] 002-make_html_ids_discoverable_v4.patch (5.2K, ../../[email protected]/3-002-make_html_ids_discoverable_v4.patch)
download | inline diff:
diff --git a/doc/src/sgml/stylesheet-html-common.xsl b/doc/src/sgml/stylesheet-html-common.xsl
index 9df2782ce4..65c58ba750 100644
--- a/doc/src/sgml/stylesheet-html-common.xsl
+++ b/doc/src/sgml/stylesheet-html-common.xsl
@@ -301,4 +301,126 @@ set toc,title
</xsl:choose>
</xsl:template>
+
+<!-- Override the standard section heading generation to add an id link -->
+<xsl:template name="section.heading">
+ <xsl:param name="section" select="."/>
+ <xsl:param name="level" select="1"/>
+ <xsl:param name="allow-anchors" select="1"/>
+ <xsl:param name="title"/>
+ <xsl:param name="class" select="'title'"/>
+ <xsl:variable name="id">
+ <xsl:choose>
+ <!-- Make sure the subtitle doesn't get the same id as the title -->
+ <xsl:when test="self::subtitle">
+ <xsl:call-template name="object.id">
+ <xsl:with-param name="object" select="."/>
+ </xsl:call-template>
+ </xsl:when>
+ <!-- if title is in an *info wrapper, get the grandparent -->
+ <xsl:when test="contains(local-name(..), 'info')">
+ <xsl:call-template name="object.id">
+ <xsl:with-param name="object" select="../.."/>
+ </xsl:call-template>
+ </xsl:when>
+ <xsl:otherwise>
+ <xsl:call-template name="object.id">
+ <xsl:with-param name="object" select=".."/>
+ </xsl:call-template>
+ </xsl:otherwise>
+ </xsl:choose>
+ </xsl:variable>
+
+ <!-- HTML H level is one higher than section level -->
+ <xsl:variable name="hlevel">
+ <xsl:choose>
+ <!-- highest valid HTML H level is H6; so anything nested deeper
+ than 5 levels down just becomes H6 -->
+ <xsl:when test="$level > 5">6</xsl:when>
+ <xsl:otherwise>
+ <xsl:value-of select="$level + 1"/>
+ </xsl:otherwise>
+ </xsl:choose>
+ </xsl:variable>
+ <xsl:element name="h{$hlevel}" namespace="http://www.w3.org/1999/xhtml">
+ <xsl:attribute name="class"><xsl:value-of select="$class"/></xsl:attribute>
+ <xsl:if test="$css.decoration != '0'">
+ <xsl:if test="$hlevel<3">
+ <xsl:attribute name="style">clear: both</xsl:attribute>
+ </xsl:if>
+ </xsl:if>
+ <xsl:if test="$allow-anchors != 0">
+ <xsl:call-template name="anchor">
+ <xsl:with-param name="node" select="$section"/>
+ <xsl:with-param name="conditional" select="0"/>
+ </xsl:call-template>
+ </xsl:if>
+ <xsl:copy-of select="$title"/>
+ <xsl:call-template name="pg.id.link">
+ <xsl:with-param name="object" select="$section"/>
+ </xsl:call-template>
+ </xsl:element>
+</xsl:template>
+
+
+<!-- Override the standard varlistentry/term generation to add an id link
+ after the last term -->
+<xsl:template match="varlistentry/term">
+ <xsl:apply-imports/>
+ <xsl:if test="position() = last()">
+ <xsl:call-template name="pg.id.link">
+ <xsl:with-param name="object" select="parent::varlistentry"/>
+ </xsl:call-template>
+ </xsl:if>
+</xsl:template>
+
+
+<!-- Creates a link pointing to an id within the document -->
+<xsl:template name="pg.id.link">
+ <xsl:param name="object" select="."/>
+ <xsl:choose>
+ <xsl:when test="$object/@id or $object/@xml:id">
+ <xsl:text> </xsl:text>
+ <a>
+ <xsl:attribute name="href">
+ <xsl:text>#</xsl:text>
+ <xsl:call-template name="object.id">
+ <xsl:with-param name="object" select="$object"/>
+ </xsl:call-template>
+ </xsl:attribute>
+ <xsl:attribute name="class">
+ <xsl:text>id_link</xsl:text>
+ </xsl:attribute>
+ <xsl:text>#</xsl:text>
+ </a>
+ </xsl:when>
+ <xsl:otherwise>
+ <!-- Only complain about varlistentries if at least one entry in
+ the list has an id -->
+ <xsl:if test="name($object) != 'varlistentry'
+ or $object/parent::variablelist/varlistentry[@id]">
+ <xsl:message terminate="yes">
+ <xsl:text> </xsl:text> <!-- leading newline -->
+ <xsl:text>Ids are required in order to provide the public</xsl:text>
+ <xsl:text> HTML documentation with stable URLs for <</xsl:text>
+ <xsl:value-of select ="name($object)"/>
+ <xsl:text>> element content; id missing at: </xsl:text>
+ <xsl:for-each select="$object/ancestor::*">
+ <xsl:text>/</xsl:text>
+ <xsl:value-of select ="name(.)"/>
+ <xsl:if test="@id|@xml:id">
+ <xsl:text>[@</xsl:text>
+ <xsl:value-of select ="name(@id|@xml:id)"/>
+ <xsl:text> = '</xsl:text>
+ <xsl:value-of select ="@id"/>
+ <xsl:text>']</xsl:text>
+ </xsl:if>
+ </xsl:for-each>
+ <xsl:text> </xsl:text> <!-- trailing newline -->
+ </xsl:message>
+ </xsl:if>
+ </xsl:otherwise>
+ </xsl:choose>
+</xsl:template>
+
</xsl:stylesheet>
diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
index cc14efa1ca..15bcc95d41 100644
--- a/doc/src/sgml/stylesheet.css
+++ b/doc/src/sgml/stylesheet.css
@@ -169,3 +169,13 @@ acronym { font-style: inherit; }
width: 75%;
}
}
+
+/* Links to ids of headers and definition terms */
+a.id_link {
+ color: inherit;
+ visibility: hidden;
+}
+
+*:hover > a.id_link {
+ visibility: visible;
+}
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-23 07:24 Brar Piening <[email protected]>
parent: Karl O. Pinc <[email protected]>
0 siblings, 1 reply; 48+ messages in thread
From: Brar Piening @ 2023-03-23 07:24 UTC (permalink / raw)
To: Karl O. Pinc <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On 23.03.2023 at 04:09, Karl O. Pinc wrote:
> You're quite right. I clearly didn't have my XSLT turned on. Importing
> only works when templates are matched, not called by name.
>
> Sorry for the extra work I've put you through.
No problem. As always I've learnt something which may help me in the future.
> You've put in a lot of good work. I'm attaching 2 patches
> with only minor changes.
Thanks. When comparing things I also realized that I had accidentally
created a reversed patch. Thanks for fixing this.
> 001-add-needed-ids_v1.patch
>
> This separates out the addition of ids from the XSLT changes, just
> to keep things tidy. Content is from your patch.
+1
> 002-make_html_ids_discoverable_v4.patch
>
> I changed the linked text, the #, so that the leading space
> is not linked. This is arguable, as the extra space makes
> it easier to put the mouse on the region. But it seems
> tidy.
I tend to prefer a slightly bigger mouseover-region but I don't really mind.
> I've tided up so the lines are no longer than 80 chars.
+1
> This looks awesome. I love the xpath! I've changed the format of the
> error message. What do you think? (Try it out by _not_ applying
> 001-add-needed-ids_v1.patch.)
>
> Also, the error message now has leading and trailing newlines to make
> it stand out. I'm normally against this sort of thing but thought I'd
> add it anyway for others to review.
+1
> I'm ready to send these on to a committer but if you don't
> like what I did please send more patches for me to review.
I like it and think it's ready for commiter.
> Outstanding questions (for committer?):
>
> The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
> <h2>, etc. attributes using a XSLT <element> element with a
> "namespace" attribute.
I'm not sure I follow. I cannot see any namespacing weirdness in my output.
Are you using the v1.79.2 styleshhets?
> What character should be used to represent a link anchor?
It's not the first time this is coming up. See my response in the old
thread:
https://www.postgresql.org/message-id/e50193ea-ca5c-e178-026a-f3fd8942252d%40gmx.de
Personally I'd advise to stick with ASCII for now.
In any case changing the symbol at some point would be a very minor
effort if we deem it necessary.
Maybe this could be part of some general overhaul of the visual
apperance and website styling by a person with more talent for this than
I have.
Regards,
Brar
^ permalink raw reply [nested|flat] 48+ messages in thread
* Re: doc: add missing "id" attributes to extension packaging page
@ 2023-03-23 12:51 Karl O. Pinc <[email protected]>
parent: Brar Piening <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Karl O. Pinc @ 2023-03-23 12:51 UTC (permalink / raw)
To: Brar Piening <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Tom Lane <[email protected]>; vignesh C <[email protected]>; Alvaro Herrera <[email protected]>; Ian Lawrence Barwick <[email protected]>; pgsql-hackers
On Thu, 23 Mar 2023 08:24:48 +0100
Brar Piening <[email protected]> wrote:
> On 23.03.2023 at 04:09, Karl O. Pinc wrote:
> > Sorry for the extra work I've put you through.
>
> No problem. As always I've learnt something which may help me in the
> future.
I don't know about you, but sadly, my brain eventually leaks. ;-)
> > I'm attaching 2 patches
> > with only minor changes.
> > 001-add-needed-ids_v1.patch
> >
> > This separates out the addition of ids from the XSLT changes, just
> > to keep things tidy.
> > 002-make_html_ids_discoverable_v4.patch
> >
> > I changed the linked text, the #, so that the leading space
> > is not linked. This is arguable, as the extra space makes
> > it easier to put the mouse on the region.
> I tend to prefer a slightly bigger mouseover-region but I don't
> really mind.
I'm leaving it for the committer to review.
> I've changed the format of
> > the error message. What do you think? (Try it out by _not_
> > applying 001-add-needed-ids_v1.patch.)
> >
> > Also, the error message now has leading and trailing newlines to
> > make it stand out.
Including the error message/make output here, so everyone can see
easily.
--------------<snip>------------
/usr/bin/xsltproc --nonet --path . --stringparam pg.version '16devel' stylesheet.xsl postgres-full.xml
Ids are required in order to provide the public HTML documentation with stable URLs for <varlistentry> element content; id missing at: /book[@id = 'postgres']/part[@id = 'appendixes']/appendix[@id = 'contrib']/sect1[@id = 'pgwalinspect']/sect2[@id = 'pgwalinspect-funcs']/variablelist
no result for postgres-full.xml
make: *** [Makefile:146: html-stamp] Error 10
--------------<snip>------------
> I like it and think it's ready for commiter.
I've marked it ready for the committer in the commitfest.
> > Outstanding questions (for committer?):
> >
> > The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
> > <h2>, etc. attributes using a XSLT <element> element with a
> > "namespace" attribute.
>
> I'm not sure I follow. I cannot see any namespacing weirdness in my
> output.
There's nothing weird in the output, it's all about how
you're generating it in the xslt with
<xsl:element name="h$level" namespace="...
Output looks right to me.
> Are you using the v1.79.2 styleshhets?
Yes. But I've got both the ones with namespaces and without
installed.
I've just never had to look at what PG is doing with namespaces
before. What you've done looks right to me, but I'm pretty
clueless so somebody else should double check.]
> > What character should be used to represent a link anchor?
> Personally I'd advise to stick with ASCII for now.
+1
Regards,
Karl <[email protected]>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 575cbcb13a3..fa628e410e6 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -312,6 +312,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -1001,7 +1002,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1253,6 +1257,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 22a2494a3f8..cc3071644c3 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 61 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 48 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f59b03222b0..5decb1127d0 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -295,6 +295,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -1002,7 +1003,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1206,6 +1210,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a3c971cd26d..295846b854f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,9 +1413,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1437,33 +1425,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
- *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1499,7 +1464,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1515,7 +1480,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1524,9 +1489,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1538,7 +1503,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1547,7 +1512,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1615,7 +1580,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 23ba23b5b01..bdde17eb230 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -218,6 +218,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0018-Initialize-xl_heap_prune-deserialization-variable.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 46b5173a401..c5046da6d1e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -298,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -987,7 +988,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1239,6 +1243,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f4bf60192f8..86524ae0c3d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
+ int lpdead_items; /* includes existing LP_DEAD items */
} PruneFreezeResult;
/* ----------------
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 575cbcb13a3..fa628e410e6 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -312,6 +312,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -1001,7 +1002,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1253,6 +1257,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 22a2494a3f8..cc3071644c3 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 61 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 48 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f59b03222b0..5decb1127d0 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -295,6 +295,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -1002,7 +1003,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1206,6 +1210,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a3c971cd26d..295846b854f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,9 +1413,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1437,33 +1425,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
- *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1499,7 +1464,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1515,7 +1480,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1524,9 +1489,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1538,7 +1503,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1547,7 +1512,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1615,7 +1580,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 23ba23b5b01..bdde17eb230 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -218,6 +218,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0018-Initialize-xl_heap_prune-deserialization-variable.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 46b5173a401..c5046da6d1e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -298,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -987,7 +988,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1239,6 +1243,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f4bf60192f8..86524ae0c3d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
+ int lpdead_items; /* includes existing LP_DEAD items */
} PruneFreezeResult;
/* ----------------
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 575cbcb13a3..fa628e410e6 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -312,6 +312,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -1001,7 +1002,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1253,6 +1257,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 22a2494a3f8..cc3071644c3 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 61 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 48 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index f59b03222b0..5decb1127d0 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -295,6 +295,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -1002,7 +1003,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1206,6 +1210,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index a3c971cd26d..295846b854f 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,9 +1413,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1437,33 +1425,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
- *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1499,7 +1464,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1515,7 +1480,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1524,9 +1489,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1538,7 +1503,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1547,7 +1512,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1615,7 +1580,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 23ba23b5b01..bdde17eb230 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -218,6 +218,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* ----------------
--
2.40.1
--tez7m2a73jtztiij
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0018-Initialize-xl_heap_prune-deserialization-variable.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune
@ 2024-01-22 21:55 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-01-22 21:55 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 ++++++----------------------
src/include/access/heapam.h | 2 +
3 files changed, 22 insertions(+), 47 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 46b5173a401..c5046da6d1e 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -298,6 +298,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Keep track of whether or not the page is all_visible in case the caller
@@ -987,7 +988,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1239,6 +1243,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 634f4da9a17..4b45e8be1ad 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1439,23 +1439,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1468,9 +1456,9 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
@@ -1480,32 +1468,10 @@ lazy_scan_prune(LVRelState *vacrel,
&pagefrz, &presult, &vacrel->offnum);
/*
- * Now scan the page to collect LP_DEAD items and check for tuples
- * requiring freezing among remaining tuples with storage. We will update
- * the VM after collecting LP_DEAD items and freezing tuples. Pruning will
- * have determined whether or not the page is all_visible and able to
- * become all_frozen.
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all_visible.
*/
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1541,7 +1507,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1557,7 +1523,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1566,9 +1532,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1580,7 +1546,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1589,7 +1555,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1657,7 +1623,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index f4bf60192f8..86524ae0c3d 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -219,6 +219,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
+ int lpdead_items; /* includes existing LP_DEAD items */
} PruneFreezeResult;
/* ----------------
--
2.40.1
--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v2-0016-Obsolete-XLOG_HEAP2_FREEZE_PAGE.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v9 15/21] Save dead tuple offsets during heap_page_prune
@ 2024-03-26 14:16 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-03-26 14:16 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 +++++++---------------------
src/include/access/heapam.h | 2 +
3 files changed, 23 insertions(+), 46 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index fde5f26bb5a..3529ea69520 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -297,6 +297,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -971,7 +972,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1175,6 +1179,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c28e786a1e0..0fb5a7dd24d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,41 +1413,21 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
* false otherwise.
+ *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all-visible.
*/
heap_page_prune_and_freeze(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
&pagefrz, &presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
- /*
- * Now scan the page to collect LP_DEAD items and update the variables set
- * just above.
- */
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1492,7 +1460,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1508,7 +1476,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1517,9 +1485,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1531,7 +1499,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1540,7 +1508,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1608,7 +1576,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 79ec4049f12..68b4d5b859c 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -241,6 +241,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* 'reason' codes for heap_page_prune_and_freeze() */
--
2.40.1
--caj67xgx3lukmr5f
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0016-move-live-tuple-accounting-to-heap_prune_chain.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v7 15/16] Save dead tuple offsets during heap_page_prune
@ 2024-03-26 14:16 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-03-26 14:16 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 +++++++---------------------
src/include/access/heapam.h | 2 +
3 files changed, 23 insertions(+), 46 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ee557c9ed35..6d5f8ba4417 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -297,6 +297,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -975,7 +976,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1179,6 +1183,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c28e786a1e0..0fb5a7dd24d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,41 +1413,21 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
* false otherwise.
+ *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all-visible.
*/
heap_page_prune_and_freeze(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
&pagefrz, &presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
- /*
- * Now scan the page to collect LP_DEAD items and update the variables set
- * just above.
- */
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1492,7 +1460,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1508,7 +1476,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1517,9 +1485,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1531,7 +1499,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1540,7 +1508,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1608,7 +1576,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 06d75f2ad04..2740eaac13e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -241,6 +241,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* 'reason' codes for heap_page_prune_and_freeze() */
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0016-move-live-tuple-accounting-to-heap_prune_chain.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v7 15/16] Save dead tuple offsets during heap_page_prune
@ 2024-03-26 14:16 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-03-26 14:16 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
---
src/backend/access/heap/pruneheap.c | 7 ++++
src/backend/access/heap/vacuumlazy.c | 60 +++++++---------------------
src/include/access/heapam.h | 2 +
3 files changed, 23 insertions(+), 46 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index ee557c9ed35..6d5f8ba4417 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -297,6 +297,7 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
presult->live_tuples = 0;
presult->recently_dead_tuples = 0;
+ presult->lpdead_items = 0;
/*
* Caller will update the VM after pruning, collecting LP_DEAD items, and
@@ -975,7 +976,10 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
if (unlikely(prstate->mark_unused_now))
heap_prune_record_unused(prstate, offnum);
else
+ {
presult->all_visible = false;
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
+ }
break;
}
@@ -1179,6 +1183,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
* all_visible.
*/
presult->all_visible = false;
+
+ /* Record the dead offset for vacuum */
+ presult->deadoffsets[presult->lpdead_items++] = offnum;
}
/*
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c28e786a1e0..0fb5a7dd24d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1396,23 +1396,11 @@ lazy_scan_prune(LVRelState *vacrel,
bool *has_lpdead_items)
{
Relation rel = vacrel->rel;
- OffsetNumber offnum,
- maxoff;
- ItemId itemid;
- int lpdead_items = 0;
PruneFreezeResult presult;
HeapPageFreeze pagefrz;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
- /*
- * maxoff might be reduced following line pointer array truncation in
- * heap_page_prune_and_freeze(). That's safe for us to ignore, since the
- * reclaimed space will continue to look like LP_UNUSED items below.
- */
- maxoff = PageGetMaxOffsetNumber(page);
-
/* Initialize pagefrz */
pagefrz.freeze_required = false;
pagefrz.FreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
@@ -1425,41 +1413,21 @@ lazy_scan_prune(LVRelState *vacrel,
* Prune all HOT-update chains and potentially freeze tuples on this page.
*
* We count the number of tuples removed from the page by the pruning step
- * in presult.ndeleted. It should not be confused with lpdead_items;
- * lpdead_items's final value can be thought of as the number of tuples
- * that were deleted from indexes.
+ * in presult.ndeleted. It should not be confused with
+ * presult.lpdead_items; presult.lpdead_items's final value can be thought
+ * of as the number of tuples that were deleted from indexes.
*
* If the relation has no indexes, we can immediately mark would-be dead
* items LP_UNUSED, so mark_unused_now should be true if no indexes and
* false otherwise.
+ *
+ * We will update the VM after collecting LP_DEAD items and freezing
+ * tuples. Pruning will have determined whether or not the page is
+ * all-visible.
*/
heap_page_prune_and_freeze(rel, buf, vacrel->vistest, vacrel->nindexes == 0,
&pagefrz, &presult, PRUNE_VACUUM_SCAN, &vacrel->offnum);
- /*
- * Now scan the page to collect LP_DEAD items and update the variables set
- * just above.
- */
- for (offnum = FirstOffsetNumber;
- offnum <= maxoff;
- offnum = OffsetNumberNext(offnum))
- {
- /*
- * Set the offset number so that we can display it along with any
- * error that occurred while processing this tuple.
- */
- vacrel->offnum = offnum;
- itemid = PageGetItemId(page, offnum);
-
-
- if (ItemIdIsDead(itemid))
- {
- deadoffsets[lpdead_items++] = offnum;
- continue;
- }
-
- }
-
vacrel->offnum = InvalidOffsetNumber;
Assert(MultiXactIdIsValid(presult.new_relminmxid));
@@ -1492,7 +1460,7 @@ lazy_scan_prune(LVRelState *vacrel,
TransactionId debug_cutoff;
bool debug_all_frozen;
- Assert(lpdead_items == 0);
+ Assert(presult.lpdead_items == 0);
if (!heap_page_is_all_visible(vacrel, buf,
&debug_cutoff, &debug_all_frozen))
@@ -1508,7 +1476,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1517,9 +1485,9 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1531,7 +1499,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += presult.live_tuples;
vacrel->recently_dead_tuples += presult.recently_dead_tuples;
@@ -1540,7 +1508,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!presult.all_visible || !(*has_lpdead_items));
@@ -1608,7 +1576,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 06d75f2ad04..2740eaac13e 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -241,6 +241,8 @@ typedef struct PruneFreezeResult
/* New value of relminmxid found by heap_page_prune_and_freeze() */
MultiXactId new_relminmxid;
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneFreezeResult;
/* 'reason' codes for heap_page_prune_and_freeze() */
--
2.40.1
--ck6erxojvlx23byk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0016-move-live-tuple-accounting-to-heap_prune_chain.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
* [PATCH v10 09/10] Save dead tuple offsets during heap_page_prune
@ 2024-03-30 05:27 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 48+ messages in thread
From: Melanie Plageman @ 2024-03-30 05:27 UTC (permalink / raw)
After heap_page_prune() returned, lazy_scan_prune() looped through all
of the offsets of LP_DEAD items which it later added to
LVRelState->dead_items. Instead take care of this when marking a line
pointer or when an existing non-removable LP_DEAD item is encountered in
heap_prune_chain().
Because deadoffsets are expected to be in order in
LVRelState->dead_items, sort the deadoffsets before saving them there.
---
src/backend/access/heap/pruneheap.c | 17 +++++++++++++
src/backend/access/heap/vacuumlazy.c | 38 +++++++++++++++++++---------
src/include/access/heapam.h | 7 +++++
3 files changed, 50 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 73fcc0081c..7f55e9c839 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -70,6 +70,13 @@ typedef struct
/* Whether or not the page makes rel truncation unsafe */
bool hastup;
+
+ /*
+ * LP_DEAD items on the page after pruning. Includes existing LP_DEAD
+ * items
+ */
+ int lpdead_items; /* includes existing LP_DEAD items */
+ OffsetNumber *deadoffsets; /* points directly to PruneResult->deadoffsets */
} PruneState;
/* Local functions */
@@ -277,6 +284,8 @@ heap_page_prune(Relation relation, Buffer buffer,
prstate.nchain_members = 0;
prstate.nchain_candidates = 0;
prstate.hastup = false;
+ prstate.lpdead_items = 0;
+ prstate.deadoffsets = presult->deadoffsets;
/*
* If we will prepare to freeze tuples, consider that it might be possible
@@ -570,6 +579,8 @@ heap_page_prune(Relation relation, Buffer buffer,
/* Copy data back to 'presult' */
presult->nnewlpdead = prstate.ndead;
presult->ndeleted = prstate.ndeleted;
+ presult->lpdead_items = prstate.lpdead_items;
+ /* the presult->deadoffsets array was already filled in */
}
@@ -881,6 +892,9 @@ heap_prune_record_dead(PruneState *prstate, OffsetNumber offnum,
prstate->nowdead[prstate->ndead] = offnum;
prstate->ndead++;
+ /* Record the dead offset for vacuum */
+ prstate->deadoffsets[prstate->lpdead_items++] = offnum;
+
/*
* If the root entry had been a normal tuple, we are deleting it, so count
* it in the result. But changing a redirect (even to DEAD state) doesn't
@@ -1026,6 +1040,9 @@ heap_prune_record_unchanged_lp_dead(PruneState *prstate, OffsetNumber offnum)
* hastup/nonempty_pages as provisional no matter how LP_DEAD items are
* handled (handled here, or handled later on).
*/
+
+ /* Record the dead offset for vacuum */
+ prstate->deadoffsets[prstate->lpdead_items++] = offnum;
}
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 212d76045e..7f1e4db55c 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1373,6 +1373,15 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
return false;
}
+static int
+OffsetNumber_cmp(const void *a, const void *b)
+{
+ OffsetNumber na = *(const OffsetNumber *) a,
+ nb = *(const OffsetNumber *) b;
+
+ return na < nb ? -1 : na > nb;
+}
+
/*
* lazy_scan_prune() -- lazy_scan_heap() pruning and freezing.
*
@@ -1416,14 +1425,12 @@ lazy_scan_prune(LVRelState *vacrel,
maxoff;
ItemId itemid;
PruneResult presult;
- int lpdead_items,
- live_tuples,
+ int live_tuples,
recently_dead_tuples;
bool all_visible;
TransactionId visibility_cutoff_xid;
uint8 actions = 0;
int64 fpi_before = pgWalUsage.wal_fpi;
- OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
Assert(BufferGetBlockNumber(buf) == blkno);
@@ -1441,7 +1448,6 @@ lazy_scan_prune(LVRelState *vacrel,
presult.pagefrz.NoFreezePageRelfrozenXid = vacrel->NewRelfrozenXid;
presult.pagefrz.NoFreezePageRelminMxid = vacrel->NewRelminMxid;
presult.pagefrz.cutoffs = &vacrel->cutoffs;
- lpdead_items = 0;
live_tuples = 0;
recently_dead_tuples = 0;
@@ -1509,7 +1515,6 @@ lazy_scan_prune(LVRelState *vacrel,
* (This is another case where it's useful to anticipate that any
* LP_DEAD items will become LP_UNUSED during the ongoing VACUUM.)
*/
- deadoffsets[lpdead_items++] = offnum;
continue;
}
@@ -1713,7 +1718,7 @@ lazy_scan_prune(LVRelState *vacrel,
*/
#ifdef USE_ASSERT_CHECKING
/* Note that all_frozen value does not matter when !all_visible */
- if (all_visible && lpdead_items == 0)
+ if (all_visible && presult.lpdead_items == 0)
{
TransactionId debug_cutoff;
bool debug_all_frozen;
@@ -1730,7 +1735,7 @@ lazy_scan_prune(LVRelState *vacrel,
/*
* Now save details of the LP_DEAD items from the page in vacrel
*/
- if (lpdead_items > 0)
+ if (presult.lpdead_items > 0)
{
VacDeadItems *dead_items = vacrel->dead_items;
ItemPointerData tmp;
@@ -1739,9 +1744,18 @@ lazy_scan_prune(LVRelState *vacrel,
ItemPointerSetBlockNumber(&tmp, blkno);
- for (int i = 0; i < lpdead_items; i++)
+ /*
+ * dead_items are expected to be in order. However, deadoffsets are
+ * collected incrementally in heap_page_prune_and_freeze() as each
+ * dead line pointer is recorded, with an indeterminate order. As
+ * such, sort the deadoffsets before saving them in LVRelState.
+ */
+ qsort(presult.deadoffsets, presult.lpdead_items, sizeof(OffsetNumber),
+ OffsetNumber_cmp);
+
+ for (int i = 0; i < presult.lpdead_items; i++)
{
- ItemPointerSetOffsetNumber(&tmp, deadoffsets[i]);
+ ItemPointerSetOffsetNumber(&tmp, presult.deadoffsets[i]);
dead_items->items[dead_items->num_items++] = tmp;
}
@@ -1766,7 +1780,7 @@ lazy_scan_prune(LVRelState *vacrel,
/* Finally, add page-local counts to whole-VACUUM counts */
vacrel->tuples_deleted += presult.ndeleted;
vacrel->tuples_frozen += presult.nfrozen;
- vacrel->lpdead_items += lpdead_items;
+ vacrel->lpdead_items += presult.lpdead_items;
vacrel->live_tuples += live_tuples;
vacrel->recently_dead_tuples += recently_dead_tuples;
@@ -1775,7 +1789,7 @@ lazy_scan_prune(LVRelState *vacrel,
vacrel->nonempty_pages = blkno + 1;
/* Did we find LP_DEAD items? */
- *has_lpdead_items = (lpdead_items > 0);
+ *has_lpdead_items = (presult.lpdead_items > 0);
Assert(!all_visible || !(*has_lpdead_items));
@@ -1843,7 +1857,7 @@ lazy_scan_prune(LVRelState *vacrel,
* There should never be LP_DEAD items on a page with PD_ALL_VISIBLE set,
* however.
*/
- else if (lpdead_items > 0 && PageIsAllVisible(page))
+ else if (presult.lpdead_items > 0 && PageIsAllVisible(page))
{
elog(WARNING, "page containing LP_DEAD items is marked as all-visible in relation \"%s\" page %u",
vacrel->relname, blkno);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 2311d01998..e346312471 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -252,6 +252,13 @@ typedef struct PruneResult
bool all_frozen;
int nfrozen;
HeapTupleFreeze frozen[MaxHeapTuplesPerPage];
+
+ /*
+ * LP_DEAD items on the page after pruning. Includes existing LP_DEAD
+ * items
+ */
+ int lpdead_items;
+ OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
} PruneResult;
/* 'reason' codes for heap_page_prune() */
--
2.40.1
--nehniapdxqvkrsct
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0010-Combine-freezing-and-pruning.patch"
^ permalink raw reply [nested|flat] 48+ messages in thread
end of thread, other threads:[~2024-03-30 05:27 UTC | newest]
Thread overview: 48+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-10-22 17:28 Multiple Wait Events for extensions legrand legrand <[email protected]>
2018-10-22 22:47 ` Michael Paquier <[email protected]>
2018-10-24 18:18 ` legrand legrand <[email protected]>
2018-10-25 01:14 ` Michael Paquier <[email protected]>
2018-10-28 16:57 ` legrand legrand <[email protected]>
2018-10-28 17:12 ` legrand legrand <[email protected]>
2018-10-29 00:05 ` Michael Paquier <[email protected]>
2023-01-09 20:18 Re: doc: add missing "id" attributes to extension packaging page Tom Lane <[email protected]>
2023-01-09 20:45 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-09 22:28 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-09 22:30 ` Re: doc: add missing "id" attributes to extension packaging page Tom Lane <[email protected]>
2023-01-10 05:28 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-10 05:12 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-10 05:22 ` Re: doc: add missing "id" attributes to extension packaging page Tom Lane <[email protected]>
2023-01-11 14:22 ` Re: doc: add missing "id" attributes to extension packaging page Peter Eisentraut <[email protected]>
2023-01-16 00:01 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-16 00:28 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-16 17:14 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-17 01:05 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-17 05:57 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-17 13:12 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-17 18:13 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-17 22:43 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-01-18 05:50 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-26 20:48 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-02-14 20:13 ` Re: doc: add missing "id" attributes to extension packaging page Andres Freund <[email protected]>
2023-02-15 19:34 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-02-15 20:49 ` Re: doc: add missing "id" attributes to extension packaging page Andres Freund <[email protected]>
2023-03-20 18:47 ` Re: doc: add missing "id" attributes to extension packaging page Gregory Stark (as CFM) <[email protected]>
2023-03-21 13:57 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-01-19 04:05 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-03-21 22:16 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-03-23 03:09 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2023-03-23 07:24 ` Re: doc: add missing "id" attributes to extension packaging page Brar Piening <[email protected]>
2023-03-23 12:51 ` Re: doc: add missing "id" attributes to extension packaging page Karl O. Pinc <[email protected]>
2024-01-22 21:55 [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v3 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v2 15/17] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-01-22 21:55 [PATCH v4 17/19] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-03-26 14:16 [PATCH v7 15/16] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-03-26 14:16 [PATCH v9 15/21] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-03-26 14:16 [PATCH v7 15/16] Save dead tuple offsets during heap_page_prune Melanie Plageman <[email protected]>
2024-03-30 05:27 [PATCH v10 09/10] Save dead tuple offsets during heap_page_prune 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