Обсуждение: Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE

While hacking on pg_rewind, I noticed that commit and abort WAL records 
are never marked with the XLR_SPECIAL_REL_UPDATE flag. But if the record 
contains "dropped relfilenodes", surely it should be?

It's harmless as far as the backend and all the programs in PostgreSQL 
repository are concerned, but the point of XLR_SPECIAL_REL_UPDATE is to 
aid external tools that try to track which files are modified. Attached 
is a patch to fix it.

It's always been like that, but I am not going backport, for fear of 
breaking existing applications. If a program reads the WAL, and would 
actually need to do something with commit records dropping relations, 
that seems like such a common scenario that the author should've thought 
about it and handled it even without the flag reminding about it. Fixing 
it in master ought to be enough.

Thoughts?

- Heikki

On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> While hacking on pg_rewind, I noticed that commit and abort WAL records
> are never marked with the XLR_SPECIAL_REL_UPDATE flag. But if the record
> contains "dropped relfilenodes", surely it should be?
>

Right.

> It's harmless as far as the backend and all the programs in PostgreSQL
> repository are concerned, but the point of XLR_SPECIAL_REL_UPDATE is to
> aid external tools that try to track which files are modified. Attached
> is a patch to fix it.
>
> It's always been like that, but I am not going backport, for fear of
> breaking existing applications. If a program reads the WAL, and would
> actually need to do something with commit records dropping relations,
> that seems like such a common scenario that the author should've thought
> about it and handled it even without the flag reminding about it. Fixing
> it in master ought to be enough.
>

+1 for doing it in master only. Even if someone comes up with such a
scenario for back-branches, we can revisit our decision to backport
this but like you, I also don't see any pressing need to do it now.


-- 
With Regards,
Amit Kapila.

On Sat, Aug 15, 2020 at 11:05:43AM +0530, Amit Kapila wrote:
> On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> It's always been like that, but I am not going backport, for fear of
>> breaking existing applications. If a program reads the WAL, and would
>> actually need to do something with commit records dropping relations,
>> that seems like such a common scenario that the author should've thought
>> about it and handled it even without the flag reminding about it. Fixing
>> it in master ought to be enough.
>
> +1 for doing it in master only. Even if someone comes up with such a
> scenario for back-branches, we can revisit our decision to backport
> this but like you, I also don't see any pressing need to do it now.

+1.
--
Michael

On 17/08/2020 10:00, Michael Paquier wrote:
> On Sat, Aug 15, 2020 at 11:05:43AM +0530, Amit Kapila wrote:
>> On Fri, Aug 14, 2020 at 2:17 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> It's always been like that, but I am not going backport, for fear of
>>> breaking existing applications. If a program reads the WAL, and would
>>> actually need to do something with commit records dropping relations,
>>> that seems like such a common scenario that the author should've thought
>>> about it and handled it even without the flag reminding about it. Fixing
>>> it in master ought to be enough.
>>
>> +1 for doing it in master only. Even if someone comes up with such a
>> scenario for back-branches, we can revisit our decision to backport
>> this but like you, I also don't see any pressing need to do it now.
> 
> +1.

Pushed, thanks!

- Heikki