On Mon, Sep 11, 2023 at 5:13 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> I'll need a bit more input from Fujii-san before doing anything about
> his comments, still it looks like a doc issue to me that may need a
> backpatch to clarify how the non-transactional case behaves.
>
I would prefer to associate the new parameter 'flush' with
non-transactional messages as per the proposed patch.
Few points for you to consider:
1.
+CREATE OR REPLACE FUNCTION pg_catalog.pg_logical_emit_message(
+ transactional boolean,
+ prefix text,
+ message text,
+ flush boolean DEFAULT false)
+RETURNS pg_lsn
+LANGUAGE INTERNAL
+VOLATILE STRICT
+AS 'pg_logical_emit_message_text';
+
+CREATE OR REPLACE FUNCTION pg_catalog.pg_logical_emit_message(
+ transactional boolean,
+ prefix text,
+ message bytea,
+ flush boolean DEFAULT false)
+RETURNS pg_lsn
+LANGUAGE INTERNAL
+VOLATILE STRICT
Is there a reason to make the functions strict now when they were not earlier?
2.
+ The <parameter>flush</parameter> parameter (default set to
+ <literal>false</literal>) controls if the message is immediately
+ flushed to WAL or not. <parameter>flush</parameter> has no effect
+ with <parameter>transactional</parameter>, as the message's WAL
+ record is flushed when its transaction is committed.
The last part of the message sounds a bit too specific (".. as the
message's WAL record is flushed when its transaction is committed.")
because sometimes the WAL could be flushed by walwriter even before
the commit. Can we say something along the lines: ".. as the message's
WAL record is flushed along with its transaction."?
3.
+ /*
+ * Make sure that the message hits disk before leaving if not emitting a
+ * transactional message, if flush is requested.
+ */
+ if (!transactional && flush)
Two ifs in the above comment sound a bit odd but if we want to keep it
like that then adding 'and' before the second if may slightly improve
it.
--
With Regards,
Amit Kapila.