public inbox for [email protected]  

help / color / mirror / Atom feed

From: Andres Freund <[email protected]>
To: Matthias van de Meent <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Extensible storage manager API - SMGR hook Redux
Date: Fri, 30 Jun 2023 18:50:07 -0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAEze2WgMySu2suO_TLvFyGY3URa4mAx22WeoEicnK=PCNWEMrA@mail.gmail.com>
References: <CAEze2WgMySu2suO_TLvFyGY3URa4mAx22WeoEicnK=PCNWEMrA@mail.gmail.com>

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

view thread (9+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Extensible storage manager API - SMGR hook Redux
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox