Re: Extensible storage manager API - SMGR hook Redux - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Extensible storage manager API - SMGR hook Redux
Date
Msg-id 20230701015007.fu5nsjaxoird3ejx@awork3.anarazel.de
Whole thread Raw
In response to Extensible storage manager API - SMGR hook Redux  (Matthias van de Meent <boekewurm+postgres@gmail.com>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Synchronizing slots from primary to standby
Next
From: Pavel Stehule
Date:
Subject: Re: RFC: pg_stat_logmsg