Message-ID: <f9cd058c-4f43-3f6c-9f3e-0339f53b8228@enterprisedb.com>
Date: Wed, 16 Aug 2023 12:01:01 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: pg_logical_emit_message() misses a XLogFlush()
Content-Language: en-US
To: Andres Freund <andres@anarazel.de>
Cc: Michael Paquier <michael@paquier.xyz>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
References: <ZNsdThSe2qgsfs7R@paquier.xyz>
 <4e680b60-9bce-7c4a-0419-f478a30d30e3@enterprisedb.com>
 <ZNv1IEHhGxrUTtwI@paquier.xyz>
 <20230816003333.7hn2rx5m2l7una3d@awork3.anarazel.de>
 <619e219c-5894-cfd0-5634-0d465c41130b@enterprisedb.com>
 <20230816041336.4tsksa2qcudfgcc6@awork3.anarazel.de>
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
In-Reply-To: <20230816041336.4tsksa2qcudfgcc6@awork3.anarazel.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/f9cd058c-4f43-3f6c-9f3e-0339f53b8228%40enterprisedb.com>
Precedence: bulk


On 8/16/23 06:13, Andres Freund wrote:
> Hi,
> 
> On 2023-08-16 03:20:53 +0200, Tomas Vondra wrote:
>> On 8/16/23 02:33, Andres Freund wrote:
>>> Hi,
>>>
>>> On 2023-08-16 06:58:56 +0900, Michael Paquier wrote:
>>>> On Tue, Aug 15, 2023 at 11:37:32AM +0200, Tomas Vondra wrote:
>>>>> Shouldn't the flush be done only for non-transactional messages? The
>>>>> transactional case will be flushed by regular commit flush.
>>>>
>>>> Indeed, that would be better.  I am sending an updated patch.
>>>>
>>>> I'd like to backpatch that, would there be any objections to that?
>>>
>>> Yes, I object. This would completely cripple the performance of some uses of
>>> logical messages - a slowdown of several orders of magnitude. It's not clear
>>> to me that flushing would be the right behaviour if it weren't released, but
>>> it certainly doesn't seem right to make such a change in a minor release.
>>>
>>
>> So are you objecting to adding the flush in general, or just to the
>> backpatching part?
> 
> Both, I think. I don't object to adding a way to trigger flushing, but I think
> it needs to be optional.
> 
> 
>> IMHO we either guarantee durability of non-transactional messages, in which
>> case this would be a clear bug - and I'd say a fairly serious one.  I'm
>> curious what the workload that'd see order of magnitude of slowdown does
>> with logical messages I've used it, but even if such workload exists, would
>> it really be enough to fix any other durability bug?
> 
> Not sure what you mean with the last sentence?
> 

Sorry, I meant to write "enough not to fix any other durability bug".

That is, either we must not lose messages, in which case it's a bug and
we should fix that. Or it's acceptable for the intended use cases, in
which case there's nothing to fix.

To me losing messages seems like a bad thing, but if the users are aware
of it and are fine with it ... I'm simply arguing that if we conclude
this is a durability bug, we should not leave it unfixed because it
might have performance impact.

> I've e.g. used non-transactional messages for:
> 
> - A non-transactional queuing system. Where sometimes one would dump a portion
>   of tables into messages, with something like
>   SELECT pg_logical_emit_message(false, 'app:<task>', to_json(r)) FROM r;
>   Obviously flushing after every row would be bad.
> 
>   This is useful when you need to coordinate with other systems in a
>   non-transactional way. E.g. letting other parts of the system know that
>   files on disk (or in S3 or ...) were created/deleted, since a database
>   rollback wouldn't unlink/revive the files.
> 
> - Audit logging, when you want to log in a way that isn't undone by rolling
>   back transaction - just flushing every pg_logical_emit_message() would
>   increase the WAL flush rate many times, because instead of once per
>   transaction, you'd now flush once per modified row. It'd basically make it
>   impractical to use for such things.
> 
> - Optimistic locking. Emitting things that need to be locked on logical
>   replicas, to be able to commit on the primary. A pre-commit hook would wait
>   for the WAL to be replayed sufficiently - but only once per transaction, not
>   once per object.
> 

How come the possibility of losing messages is not an issue for these
use cases? I mean, surely auditors would not like that, and I guess
forgetting locks might be bad too.

> 
>> Or perhaps we don't want to guarantee durability for such messages, in
>> which case we don't need to fix it at all (even in master).
> 
> Well, I can see adding an option to flush, or perhaps a separate function to
> flush, to master.
>>
>> The docs are not very clear on what to expect, unfortunately. It says
>> that non-transactional messages are "written immediately" which I could
>> interpret in either way.
> 
> Yea, the docs certainly should be improved, regardless what we end up with.
> 

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company