Re: history file on replica and double switchover - Mailing list pgsql-hackers

Fujii Masao, David Zhang, Anastasia Lubennikova, many thanks to you for 
efforts with this patch!
Can I mark it as ready for committer?

On 9/25/20 10:07 AM, Fujii Masao wrote:
>
>
> On 2020/09/25 13:00, David Zhang wrote:
>> On 2020-09-24 5:00 p.m., Fujii Masao wrote:
>>>
>>>
>>> On 2020/09/25 8:15, David Zhang wrote:
>>>> Hi,
>>>>
>>>> My understanding is that the "archiver" won't even start if 
>>>> "archive_mode = on" has been set on a "replica". Therefore, either 
>>>> (XLogArchiveMode == ARCHIVE_MODE_ALWAYS) or (XLogArchiveMode != 
>>>> ARCHIVE_MODE_OFF) will produce the same result.
>>>
>>> Yes, the archiver isn't invoked in the standby when archive_mode=on.
>>> But, with the original patch, walreceiver creates .ready archive 
>>> status file
>>> even when archive_mode=on and no archiver is running. This causes
>>> the history file to be archived after the standby is promoted and
>>> the archiver is invoked.
>>>
>>> With my patch, walreceiver creates .ready archive status for the 
>>> history file
>>> only when archive_mode=always, like it does for the streamed WAL files.
>>> This is the difference between those two patches. To prevent the 
>>> streamed
>>> timeline history file from being archived, IMO we should use the 
>>> condition
>>> archive_mode==always in the walreceiver.
>> +1
>>>
>>>
>>>>
>>>> Please see how the "archiver" is started in 
>>>> src/backend/postmaster/postmaster.c
>>>>
>>>> 5227                 /*
>>>> 5228                  * Start the archiver if we're responsible for 
>>>> (re-)archiving received
>>>> 5229                  * files.
>>>> 5230                  */
>>>> 5231                 Assert(PgArchPID == 0);
>>>> 5232                 if (XLogArchivingAlways())
>>>> 5233                         PgArchPID = pgarch_start();
>>>>
>>>> I did run the nice script "double_switchover.sh" using either 
>>>> "always" or "on" on patch v1 and v2. They all generate the same 
>>>> results below. In other words, whether history file 
>>>> (00000003.history) is archived or not depends on "archive_mode" 
>>>> settings.
>>>>
>>>> echo "archive_mode = always" >> ${PGDATA_NODE2}/postgresql.auto.conf
>>>>
>>>> $ ls -l archive
>>>> -rw------- 1 david david 16777216 Sep 24 14:40 
>>>> 000000010000000000000002
>>>> ... ...
>>>> -rw------- 1 david david 16777216 Sep 24 14:40 
>>>> 00000002000000000000000A
>>>> -rw------- 1 david david       41 Sep 24 14:40 00000002.history
>>>> -rw------- 1 david david       83 Sep 24 14:40 00000003.history
>>>>
>>>> echo "archive_mode = on" >> ${PGDATA_NODE2}/postgresql.auto.conf
>>>>
>>>> $ ls -l archive
>>>> -rw------- 1 david david 16777216 Sep 24 14:47 
>>>> 000000010000000000000002
>>>> ... ...
>>>> -rw------- 1 david david 16777216 Sep 24 14:47 
>>>> 00000002000000000000000A
>>>> -rw------- 1 david david       41 Sep 24 14:47 00000002.history
>>>>
>>>>
>>>> Personally, I prefer patch v2 since it allies to the statement 
>>>> here, 
>>>> https://www.postgresql.org/docs/13/warm-standby.html#CONTINUOUS-ARCHIVING-IN-STANDBY
>>>>
>>>> "If archive_mode is set to on, the archiver is not enabled during 
>>>> recovery or standby mode. If the standby server is promoted, it 
>>>> will start archiving after the promotion, but will not archive any 
>>>> WAL it did not generate itself."
>>>>
>>>> By the way, I think the last part of the sentence should be changed 
>>>> to something like below:
>>>>
>>>> "but will not archive any WAL, which was not generated by itself."
>>>
>>> I'm not sure if this change is an improvement or not. But if we apply
>>> the patch I proposed, maybe we should mention also history file here.
>>> For example, "but will not archive any WAL or timeline history files
>>> that it did not generate itself".
>> make sense for me
>
> So I included this change in the patch. Patch attached.
>
> Regards,
>
-- 
Grigory Smolkin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company