Hi,
On Sun, Mar 27, 2022 at 09:07:12AM +0530, Bharath Rupireddy wrote:
> On Fri, Mar 25, 2022 at 10:20 AM Greg Stark <stark@mit.edu> wrote:
> >
> > This doesn't seem to be getting any further attention. It sounds like
> > Julien didn't agree with the scope of the text. Bharath do you think
> > Julien's comments make sense? Will you have a chance to look at this?
>
> Thanks Greg. I was busy with other features.
>
> Thanks Julien for the off-list discussion. I tried to address review
> comments in the v3 patch attached. Now, I've added the notes in
> high-availability.sgml which sort of suits more and closer to physical
> replicatioin than contrib.sgml or extend.sgml.
+ [...] Firstly, the module's shared library must be present on
+ both primary and standbys.
I'm a bit confused with it. It looks like it means that the .so must be
physically present on both servers, but I'm assuming that you're talking about
shared_preload_libraries?
If yes, I still think it's worth documenting that it *needs* to be present on
the standbys *if* you want it to be enabled on the standby, including if it can
be promoted to a primary node. And that any related GUC also has to be
properly configured on all nodes (so maybe moving the last paragraph just after
this one?).
If no, maybe just saying that the module has to be installed and configured on
all nodes?
+ [...] If the module exposes SQL functions, running
+ <link linkend="sql-createextension"><command>CREATE EXTENSION</command></link>
+ command on primary is sufficient as standbys will receive it via physical
+ replication.
I think it's better to phrase it with something like "CREATE EXTENSION is
replicated in physical replication similarly to other DDL commands".
+ [...] The
+ module's shared library gets loaded upon first usage of any of its
+ functions on primary and standbys.
Is it worth documenting that? Note that this is only true if the lib isn't in
shared_preload_libraries and if it's a wrapper on top of a C function.
nitpicking: there's a trailing whitespace after "standbys."
+ If the module doesn't expose SQL functions, the shared library has to be
+ loaded separately on primary and standbys, either by
+ <link linkend="sql-load"><command>LOAD</command></link> command or by
+ setting parameter <xref linkend="guc-session-preload-libraries"/> or
+ <xref linkend="guc-shared-preload-libraries"/> or
+ <xref linkend="guc-local-preload-libraries"/>, depending on module's need.
I think this is also confusing. The need for preloading is entirely orthogonal
to SQL functions in the extension, especially since this is implying SQL
function over C-code. This should be reworded to go with the first paragraph I
think.
