Re: pg_logical_emit_message() misses a XLogFlush() - Mailing list pgsql-hackers

On 2023-09-11 14:02, Michael Paquier wrote:
> On Mon, Sep 11, 2023 at 12:54:11PM +0900, Fujii Masao wrote:
>> However, in the current patch, if "transactional" is set to false and
>> "flush" is true, the function flushes the WAL immediately without
>> considering synchronous_commit. Is this the intended behavior?
>> I'm not sure how the function should work in this case, though.
> 
> Yes, that's the intended behavior.  This just offers more options to
> the toolkit of this function to give more control to applications when
> emitting a message.  In this case, like the current non-transactional
> case, we make the record immediately available to logical decoders but
> also make sure that it is flushed to disk.  If one wants to force the
> record's presence to a remote instance, then using the transactional
> mode would be sufficient.
> 
> Perhaps you have a point here, though, that we had better make
> entirely independent the flush and transactional parts, and still
> call XLogFlush() even in transactional mode.  One would make sure that
> the record is on disk before waiting for the commit to do so, but
> that's also awkward for applications because they would not know the
> end LSN of the emitted message until the internal transaction commits
> the allocated XID, which would be a few records after the result
> coming out of pg_logical_emit_message().
> 
> The documentation does not worry about any of that even now in the
> case of the non-transactional case, and it does not mention that one
> may need to monitor pg_stat_replication or similar to make sure that
> the LSN of the message exists on the remote with an application-level
> check, either.  How about adding an extra paragraph to the
> documentation, then?  I could think of something like that, but the
> current docs also outline this a bit by telling that the message is
> *not* part of a transaction, which kind of implies, at least to me,
> that synchonous_commit is moot in this case:
> "When transactional is false, note that the backend ignores
> synchronous_commit as the record is not part of a transaction so there
> is no commit to wait for. Ensuring that the record of a message
> emitted exists on standbys requires additional monitoring."
> 
>> Though I don't understand the purpose of this option fully yet,
>> is flushing the WAL sufficient? Are there scenarios where the function
>> should ensure that the WAL is not only flushed but also replicated
>> to the standby?
> 
> The flush makes sure that the record is durable, but we only care
> about transaction commits in a synchronous setup, so that's
> independent, in my opinion.  If you look closely, we do some fancy
> stuff in finish_sync_worker(), for example, where a transaction commit
> is enforced to make sure that the internal flush is sensitive to the
> synchronous commit requirements, but that's just something we expect
> to happen in a sync worker.
> --
> Michael
Hi,

With regard to the patch, the documentation outlines the 
pg_logical_emit_message function and its corresponding syntax in the 
following manner.

pg_logical_emit_message ( transactional boolean, prefix text, content 
text ) → pg_lsn
pg_logical_emit_message ( transactional boolean, prefix text, content 
bytea [, flush boolean DEFAULT false] ) → pg_lsn

A minor issue with the description here is that while the description 
for the new flush argument in pg_logical_emit_message() with bytea type 
is clearly declared, there is no description of flush argument in the 
former pg_logical_emit_message() with text type at all.

Additionally, there is a lack of consistency in the third argument names 
between the function definition and the description (i.e., "message 
bytea" versus "<parameter>content</parameter> <type>bytea</type>") as 
follows.
----------------
+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
+AS 'pg_logical_emit_message_bytea';
----------------
...
----------------
+       <function>pg_logical_emit_message</function> ( 
<parameter>transactional</parameter> <type>boolean</type>, 
<parameter>prefix</parameter> <type>text</type>, 
<parameter>content</parameter> <type>bytea</type> [, 
<parameter>flush</parameter> <type>boolean</type> 
<literal>DEFAULT</literal> <literal>false</literal>] )
----------------
Could you please provide clarification on the reason for this 
differentiation?

On a side note, could you also include a bit more information that 
"flush is set to false by default" in the document as well? It could be 
helpful for the users.

Regards,
Tung Nguyen