Re: recovery modules

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Nathan Bossart <nathandbossart(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: recovery modules
Date: 2023-01-30 07:51:38
Message-ID: Y9d3Cmk0Vjg8cYmO@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 27, 2023 at 10:27:29PM -0800, Nathan Bossart wrote:
> Here is a work-in-progress patch set for adjusting the archive modules
> interface. Is this roughly what you had in mind?

I have been catching up with what is happening here. I can get
behind the idea to use the term "callbacks" vs "context" for clarity,
to move the callback definitions into their own header, and to add
extra arguments to the callback functions for some private data.

-void
-_PG_archive_module_init(ArchiveModuleCallbacks *cb)
+const ArchiveModuleCallbacks *
+_PG_archive_module_init(void **arg)
{
AssertVariableIsOfType(&_PG_archive_module_init, ArchiveModuleInit);

- cb->check_configured_cb = basic_archive_configured;
- cb->archive_file_cb = basic_archive_file;
+ (*arg) = (void *) AllocSetContextCreate(TopMemoryContext,
+ "basic_archive",
+ ALLOCSET_DEFAULT_SIZES);
+
+ return &basic_archive_callbacks;

Now, I find this part, where we use a double pointer to allow the
module initialization to create and give back a private area, rather
confusing, and I think that this could be bug-prone, as well. Once
you incorporate some data within the set of callbacks, isn't this
stuff close to a "state" data, or just something that we could call
only an "ArchiveModule"? Could it make more sense to have
_PG_archive_module_init return a structure with everything rather than
a separate in/out argument? Here is the idea, simply:
typedef struct ArchiveModule {
ArchiveCallbacks *routines;
void *private_data;
/* Potentially more here, like some flags? */
} ArchiveModule;

That would be closer to the style of xlogreader.h, for example.

All these choices should be documented in archive_module.h, at the
end.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2023-01-30 08:16:46 Re: Syncrep and improving latency due to WAL throttling
Previous Message Peter Eisentraut 2023-01-30 07:37:42 meson: Optionally disable installation of test modules