Message-ID: <6273ef0b-033b-4ac7-ab05-71a04e7ac626@pgbackrest.org>
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Fix logging for invalid recovery timeline
To: Michael Paquier <michael@paquier.xyz>
Cc: =?UTF-8?Q?Benoit_Lobr=C3=A9au?= <benoit.lobreau@dalibo.com>,
 "Andrey M. Borodin" <x4mmm@yandex-team.ru>,
 Pg Hackers <pgsql-hackers@postgresql.org>
References: <c3d617d4-1696-4aa7-8a4d-5a7d19cc5618@pgbackrest.org>
 <C748635E-B2FF-4813-B035-AE670832B9FD@yandex-team.ru>
 <d0b27c08-d30f-4a1d-990d-5c90ca0a650a@pgbackrest.org>
 <4ed10bc8-5513-4d8e-8643-8abcaa08336d@dalibo.com>
 <2765f538-b06f-4e57-bcb2-9ca1dc6f487d@pgbackrest.org>
 <Z7aJU2_ZAplbpTsL@paquier.xyz>
Content-Language: en-US
From: David Steele <david@pgbackrest.org>
In-Reply-To: <Z7aJU2_ZAplbpTsL@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Date: Thu, 20 Feb 2025 15:40:47 +0000 (UTC)
Archived-At: 
 <https://www.postgresql.org/message-id/6273ef0b-033b-4ac7-ab05-71a04e7ac626%40pgbackrest.org>
Precedence: bulk

On 2/19/25 19:45, Michael Paquier wrote:
> On Wed, Feb 19, 2025 at 05:35:18PM +0000, David Steele wrote:
>> I like this idea but I would prefer to get the patch committed as-is first.
>> The reason is that I'm hoping to see this batch-patched (since it is a bug)
>> and that is less likely if the message wording is change.
> 
> (Had this thread flagged as a TODO for some time, sorry for not
> chiming in earlier.)

No worries. Thank you for having a look.

>> Your idea would be perfect going forward, though.
> 
> We have a few logs that already track this information, but perhaps
> that's better to track this extra element in the FATAL if you have
> log_min_messages at fatal where LOG would not show up?  Feel free to
> propose a separate patch if you think that this can be improved.

Benoit -- this was your idea. Did you want to submit a patch yourself?

> At a27048cbcb58, the check is done based on the copy of the checkpoint
> record, and the log is generated based on the checkpoint data in the
> control file.  4a92a1c3d1c3 has begun retrieving the replay TLI from
> the backup label.  v13 and v14 have this issue, but I'm not really
> tempted to poke at the beast more than necessary as this code had a
> lot of changes in the last couple of years, with few to no complaints
> as far as I am aware.

Fair enough -- this is subtle enough that I doubt almost anyone is going 
to notice and the general content of the error message is not really 
changed by the incorrect values.

> Applied down to v15 where we have xlogrecovery.c, then.  Thanks for
> the report!

Thank you for the commit!

Regards,
-David