Horton

Thread

Commits

  1. Mark commit and abort WAL records with XLR_SPECIAL_REL_UPDATE.

  1. Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE

    Heikki Linnakangas <hlinnaka@iki.fi> — 2020-08-14T08:47:24Z

    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

  2. Re: Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE

    Amit Kapila <amit.kapila16@gmail.com> — 2020-08-15T05:35:43Z

    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.

  3. Re: Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE

    Michael Paquier <michael@paquier.xyz> — 2020-08-17T07:00:17Z

    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

  4. Re: Commit/abort WAL records with dropped rels missing XLR_SPECIAL_REL_UPDATE

    Heikki Linnakangas <hlinnaka@iki.fi> — 2020-08-17T07:55:13Z

    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