Re: Extensible storage manager API - SMGR hook Redux

Hi,

On 2023-06-30 14:26:44 +0200, Matthias van de Meent wrote:
> diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
> index 4c49393fc5..8685b9fde6 100644
> --- a/src/backend/postmaster/postmaster.c
> +++ b/src/backend/postmaster/postmaster.c
> @@ -1002,6 +1002,11 @@ PostmasterMain(int argc, char *argv[])
> */
> ApplyLauncherRegister();
>
> + /*
> + * Register built-in managers that are not part of static arrays
> + */
> + register_builtin_dynamic_managers();
> +
> /*
> * process any libraries that should be preloaded at postmaster start
> */

That doesn't strike me as a good place to initialize this, we'll need it in
multiple places that way. How about putting it into BaseInit()?

> -static const f_smgr smgrsw[] = {
> +static f_smgr *smgrsw;

This adds another level of indirection. I would rather limit the number of
registerable smgrs than do that.

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

> + MemoryContextSwitchTo(old);
> +
> + pg_compiler_barrier();

Huh, what's that about?

> @@ -59,14 +63,8 @@ typedef struct SMgrRelationData
> * Fields below here are intended to be private to smgr.c and its
> * submodules. Do not touch them from elsewhere.
> */
> - int smgr_which; /* storage manager selector */
> -
> - /*
> - * for md.c; per-fork arrays of the number of open segments
> - * (md_num_open_segs) and the segments themselves (md_seg_fds).
> - */
> - int md_num_open_segs[MAX_FORKNUM + 1];
> - struct _MdfdVec *md_seg_fds[MAX_FORKNUM + 1];
> + SMgrId smgr_which; /* storage manager selector */
> + int smgrrelation_size; /* size of this struct, incl. smgr-specific data */

It looked to me like you determined this globally - why do we need it in every
entry then?

Greetings,

Andres Freund