Re: Extensible storage manager API - SMGR hook Redux

From: Kirill Reshke <reshkekirill(at)gmail(dot)com>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: Nitin Jadhav <nitinjadhavpostgres(at)gmail(dot)com>, Tristan Partin <tristan(at)neon(dot)tech>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: Extensible storage manager API - SMGR hook Redux
Date: 2025-03-10 12:30:23
Message-ID: CALdSSPimrJWeex1RbvVXoGCROLiC6VgKUdEE0pUcib=GNYo58g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 7 Mar 2025 at 16:52, Andreas Karlsson <andreas(at)proxel(dot)se> wrote:
>
> Hi,

Hi!

> Here is a rebased version of it to make the CI happy.

Looks like CI is still unhappy with this change[0]

0001:

>+
>+SMgrId
>+smgr_register(const f_smgr *smgr, Size smgrrelation_size)
...

> + Assert(smgr->smgr_open != NULL);
> + Assert(smgr->smgr_close != NULL);
> + Assert(smgr->smgr_create != NULL);
> + Assert(smgr->smgr_exists != NULL);
> + Assert(smgr->smgr_unlink != NULL);
> + Assert(smgr->smgr_extend != NULL);
> + Assert(smgr->smgr_zeroextend != NULL);
> + Assert(smgr->smgr_prefetch != NULL);
> + Assert(smgr->smgr_readv != NULL);
> + Assert(smgr->smgr_writev != NULL);
> + Assert(smgr->smgr_writeback != NULL);
> + Assert(smgr->smgr_nblocks != NULL);
> + Assert(smgr->smgr_truncate != NULL);
> + Assert(smgr->smgr_immedsync != NULL);

Are we sure we need to force extension authors to implement prefetch?
Also, do we intentionally skip Assert on smgr_registersync and
smgr_init here? I am not questioning smgr_shutdown here, as I can see
it is NULL for md implementation.

0002:
should we merge this with 0001?

0003: Looks mature, no comments.

0004:
It's a bit strange to place fsync_checker under contrib, huh? Like,
you will never use it in production. Maybe src/test/modules is a
better place?

0005:
We are missing rationale for this change in the commit message.

I didn't look at the 0006 modifications. Later, I'll try to take another look.

[0] https://cirrus-ci.com/task/6466113875214336
--
Best regards,
Kirill Reshke

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2025-03-10 12:31:51 Re: encode/decode support for base64url
Previous Message Peter Eisentraut 2025-03-10 12:27:07 Re: pg_attribute_noreturn(), MSVC, C11