public inbox for [email protected]
help / color / mirror / Atom feedRe: [PATCH] Refactor SLRU to always use long file names
2+ messages / 2 participants
[nested] [flat]
* Re: [PATCH] Refactor SLRU to always use long file names
@ 2024-09-11 23:55 Michael Paquier <[email protected]>
2024-09-12 09:33 ` Re: [PATCH] Refactor SLRU to always use long file names Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Michael Paquier @ 2024-09-11 23:55 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Wed, Sep 11, 2024 at 04:07:06PM +0300, Aleksander Alekseev wrote:
> Commit 4ed8f0913bfd introduced long SLRU file names. The proposed
> patch removes SlruCtl->long_segment_names flag and makes SLRU always
> use long file names. This simplifies both the code and the API.
> Corresponding changes to pg_upgrade are included.
That's leaner, indeed.
> One drawback I see is that technically SLRU is an exposed API and
> changing it may affect third-party code. I'm not sure if we should
> seriously worry about this. Firstly, the change is trivial and
> secondly, it's not clear whether such third-party code even exists (we
> broke this API just recently in 4ed8f0913bfd and no one complained).
Any third-party code using custom SLRUs would need to take care of
handling their upgrade path outside pg_upgrade. Not sure there are
any of them, TBH, but let's see.
> I didn't include any tests for the new pg_upgrade code. To my
> knowledge we test it manually, with buildfarm members and during
> alpha- and beta-testing periods. Please let me know if you think there
> should be a corresponding TAP test.
Removing the old API means that it is impossible to test a move from
short to long file names. That's OK by me to rely on the pg_upgrade
paths in the buildfarm code. We have a few of them.
There is one thing I am wondering, here, though, which is to think
harder about a validity check at the end of 002_pg_upgrade.pl to make
sure that all the SLRU use long file names after running the tests.
That would mean thinking about a mechanism to list all of them from a
backend, rather than hardcode a list of them. Perhaps that's not
worth it, just dropping an idea in the bucket of ideas. I would guess
in the shape of a catalog that's able to represent at SQL level all
the SLRUs that exist in a backend.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: [PATCH] Refactor SLRU to always use long file names
2024-09-11 23:55 Re: [PATCH] Refactor SLRU to always use long file names Michael Paquier <[email protected]>
@ 2024-09-12 09:33 ` Aleksander Alekseev <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Aleksander Alekseev @ 2024-09-12 09:33 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>; +Cc: Michael Paquier <[email protected]>
Hi Michael,
> On Wed, Sep 11, 2024 at 04:07:06PM +0300, Aleksander Alekseev wrote:
> > Commit 4ed8f0913bfd introduced long SLRU file names. The proposed
> > patch removes SlruCtl->long_segment_names flag and makes SLRU always
> > use long file names. This simplifies both the code and the API.
> > Corresponding changes to pg_upgrade are included.
>
> That's leaner, indeed.
>
> > One drawback I see is that technically SLRU is an exposed API and
> > changing it may affect third-party code. I'm not sure if we should
> > seriously worry about this. Firstly, the change is trivial and
> > secondly, it's not clear whether such third-party code even exists (we
> > broke this API just recently in 4ed8f0913bfd and no one complained).
>
> Any third-party code using custom SLRUs would need to take care of
> handling their upgrade path outside pg_upgrade. Not sure there are
> any of them, TBH, but let's see.
>
> > I didn't include any tests for the new pg_upgrade code. To my
> > knowledge we test it manually, with buildfarm members and during
> > alpha- and beta-testing periods. Please let me know if you think there
> > should be a corresponding TAP test.
>
> Removing the old API means that it is impossible to test a move from
> short to long file names. That's OK by me to rely on the pg_upgrade
> paths in the buildfarm code. We have a few of them.
Thanks for the feedback.
> There is one thing I am wondering, here, though, which is to think
> harder about a validity check at the end of 002_pg_upgrade.pl to make
> sure that all the SLRU use long file names after running the tests.
> That would mean thinking about a mechanism to list all of them from a
> backend, rather than hardcode a list of them. Perhaps that's not
> worth it, just dropping an idea in the bucket of ideas. I would guess
> in the shape of a catalog that's able to represent at SQL level all
> the SLRUs that exist in a backend.
Hmm... IMO it would be a rather niche facility to maintain in PG core.
At least I'm not aware of cases when a DBA wanted to list initialized
SLRUs. Would it be convenient for core / extensions developers?
Creating a breakpoint on SimpleLruInit() or adding a temporary elog()
sounds simpler to me.
It wouldn't hurt re-checking the segment file names in the TAP test
but this would mean hardcoding catalog names which as I understand you
want to avoid. With high probability PG wouldn't start if the
corresponding piece of pg_upgrade is wrong (I checked more than once
:). So I'm not entirely sure if it's worth the effort, but let's see
what others think.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-09-12 09:33 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-09-11 23:55 Re: [PATCH] Refactor SLRU to always use long file names Michael Paquier <[email protected]>
2024-09-12 09:33 ` Aleksander Alekseev <[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