Duplicate history file?

> Greetings,
>
> * Kyotaro Horiguchi ( ) wrote:
> > So, this is the new new thread.
>
> This is definitely not the way I would recommend starting up a new
> thread as you didn't include the actual text of the prior discussion for
> people to be able to read and respond to, instead making them go hunt
> for the prior discussion on the old thread and negating the point of
> starting a new thread..

> * Kyotaro Horiguchi ( ) wrote:
> > At Mon, 31 May 2021 11:52:05 +0900, Tatsuro Yamada < > wrote in
> > > Since the above behavior is different from the behavior of the
> > > test command in the following example in postgresql.conf, I think
> > > we should write a note about this example.
> > >
> > > # e.g. 'test ! -f /mnt/server/archivedir/%f && cp %p
> > > # /mnt/server/archivedir/%f'
> > >
> > > Let me describe the problem we faced.
> > > - When archive_mode=always, archive_command is (sometimes) executed
> > >   in a situation where the history file already exists on the standby
> > >   side.
> > >
> > > - In this case, if "test ! -f" is written in the archive_command of
> > >   postgresql.conf on the standby side, the command will keep failing.
> > >
> > >   Note that this problem does not occur when archive_mode=on.
> > >
> > > So, what should we do for the user? I think we should put some notes
> > > in postgresql.conf or in the documentation. For example, something
> > > like this:
>
> First off, we should tell them to not use test or cp in their actual
> archive command because they don't do things like make sure that the WAL
> that's been archived has actually been fsync'd.  Multiple people have
> tried to make improvements in this area but the long and short of it is
> that trying to provide a simple archive command in the documentation
> that actually *works* isn't enough- you need a real tool.  Maybe someone
> will write one some day that's part of core, but it's not happened yet
> and instead there's external solutions which actually do the correct
> things.

> (Mmm. thunderbird or gmail connects this thread to the previous one..)
>
> At Mon, 7 Jun 2021 14:20:38 -0400, Stephen Frost < > wrote in
> > Greetings,
> >
> > * Kyotaro Horiguchi ( ) wrote:
> > > So, this is the new new thread.
> >
> > This is definitely not the way I would recommend starting up a new
> > thread as you didn't include the actual text of the prior discussion for
> > people to be able to read and respond to, instead making them go hunt
> > for the prior discussion on the old thread and negating the point of
> > starting a new thread..
>
> Sorry for that. I'll do that next time.
>
> > Still, I went and found the other email-
>
> Thanks!
>
> > * Kyotaro Horiguchi ( ) wrote:
> > > At Mon, 31 May 2021 11:52:05 +0900, Tatsuro Yamada < > wrote in
> > > > Since the above behavior is different from the behavior of the
> > > > test command in the following example in postgresql.conf, I think
> > > > we should write a note about this example.
> > > >
> > > > # e.g. 'test ! -f /mnt/server/archivedir/%f && cp %p
> > > > # /mnt/server/archivedir/%f'
> > > >
> > > > Let me describe the problem we faced.
> > > > - When archive_mode=always, archive_command is (sometimes) executed
> > > >   in a situation where the history file already exists on the standby
> > > >   side.
> > > >
> > > > - In this case, if "test ! -f" is written in the archive_command of
> > > >   postgresql.conf on the standby side, the command will keep failing.
> > > >
> > > >   Note that this problem does not occur when archive_mode=on.
> > > >
> > > > So, what should we do for the user? I think we should put some notes
> > > > in postgresql.conf or in the documentation. For example, something
> > > > like this:
> >
> > First off, we should tell them to not use test or cp in their actual
> > archive command because they don't do things like make sure that the WAL
> > that's been archived has actually been fsync'd.  Multiple people have
> > tried to make improvements in this area but the long and short of it is
> > that trying to provide a simple archive command in the documentation
> > that actually *works* isn't enough- you need a real tool.  Maybe someone
> > will write one some day that's part of core, but it's not happened yet
> > and instead there's external solutions which actually do the correct
> > things.
>
> Ideally I agree that it is definitely right. But the documentation
> doesn't say a bit of "don't use the simple copy command in any case
> (or at least the cases where more than a certain level of durability
> and integrity guarantee is required).".
>
> Actually many people are satisfied with just "cp/copy" and I think
> they know that the command doesn't guarantee on the integrity of
> archived files on , say, some disastrous situation like a sudden power
> cut.
>
> However, the use of "test !  -f..." is in a bit different kind of
> suggestion.
>
> https://www.postgresql.org/docs/13/continuous-archiving.html
> | The archive command should generally be designed to refuse to
> | overwrite any pre-existing archive file. This is an important safety
> | feature to preserve the integrity of your archive in case of
> | administrator error (such as sending the output of two different
> | servers to the same archive directory)
>
> This implies that no WAL segment are archived more than once at least
> under any valid operation. Some people are following this suggestion
> to prevent archive from breaking by some *wrong* operations.
>
> > The existing documentation should be taken as purely "this is how the
> > variables which are passed in get expanded" not as "this is what you
> > should do", because it's very much not the latter in any form.
>

> This thread should have been started here:
>
>
>>
>> (To recap: In a replication set using archive, startup tries to
>> restore WAL files from archive before checking pg_wal directory for
>> the desired file.  The behavior itself is intentionally designed and
>> reasonable. However, the restore code notifies of a restored file
>> regardless of whether it has been already archived or not.  If
>> archive_command is written so as to return error for overwriting as we
>> suggest in the documentation, that behavior causes archive failure.)
>>
>> After playing with this, I see the problem just by restarting a
>> standby even in a simple archive-replication set after making
>> not-special prerequisites.  So I think this is worth fixing.
>>
>> With this patch, KeepFileRestoredFromArchive compares the contents of
>> just-restored file and the existing file for the same segment only
>> when:
>>
>>       - archive_mode = always
>>   and - the file to restore already exists in pgwal
>>   and - it has a .done and/or .ready status file.
>>
>> which doesn't happen usually.  Then the function skips archive
>> notification if the contents are identical.  The included TAP test is
>> working both on Linux and Windows.
>
>
> Thank you for the analysis and the patch.
> I'll try the patch tomorrow.
>
> I just noticed that this thread is still tied to another thread
> (it's not an independent thread). To fix that, it may be better to
> create a new thread again.

I understand that.

> Hi
>
>> Thank you for fixing the patch.
>> The new patch works well in my environment. :-D
>
> This may not be important at this time since it is a
> PoC patch, but I would like to inform you that there
> was a line that contained multiple spaces instead of tabs.
>
> $ git diff --check
> src/backend/access/transam/xlogarchive.c:465: trailing whitespace.
> +

> On 2021/06/09 15:58, Tatsuro Yamada wrote:
>> This may not be important at this time since it is a
>> PoC patch, but I would like to inform you that there
>> was a line that contained multiple spaces instead of tabs.
>>
>> $ git diff --check
>> src/backend/access/transam/xlogarchive.c:465: trailing whitespace.
>> +
>
> Even with the patch, if "test ! -f ..." is used in archive_command,
> you may still *easily* get the trouble that WAL archiving keeps failing?

> Hi,
>
> On 2021/06/09 16:23, Fujii Masao wrote:
> > On 2021/06/09 15:58, Tatsuro Yamada wrote:
> >> This may not be important at this time since it is a
> >> PoC patch, but I would like to inform you that there
> >> was a line that contained multiple spaces instead of tabs.
> >>
> >> $ git diff --check
> >> src/backend/access/transam/xlogarchive.c:465: trailing whitespace.
> >> +
> > Even with the patch, if "test ! -f ..." is used in archive_command,
> > you may still *easily* get the trouble that WAL archiving keeps
> > failing?

> > Instead, we should consider and document "better" command for
> > archive_command, or implement something like pg_archivecopy command
> > into the core (as far as I remember, there was the discussion about
> > this feature before...)?
>
>
> I agree with that idea.
> Since archiving is important for all users, I think there should be
> either a better and safer command in the documentation, or an archive
> command (pg_archivecopy?) that we provide as a community, as you said.
> I am curious about the conclusions of past discussions. :)

> Greetings,
>
> * Kyotaro Horiguchi ( ) wrote:
> > At Wed, 09 Jun 2021 16:56:14 +0900, Tatsuro Yamada < > wrote in
> > > On 2021/06/09 16:23, Fujii Masao wrote:
> > > > Instead, we should consider and document "better" command for
> > > > archive_command, or implement something like pg_archivecopy command
> > > > into the core (as far as I remember, there was the discussion about
> > > > this feature before...)?
> > >
> > > I agree with that idea.
> > > Since archiving is important for all users, I think there should be
> > > either a better and safer command in the documentation, or an archive
> > > command (pg_archivecopy?) that we provide as a community, as you said.
> > > I am curious about the conclusions of past discussions. :)
> >
> > How perfect the officially-provided script or command need to be?  The
> > reason that the script in the documentation is so simple is, I guess,
> > we don't/can't offer a steps sufficiently solid for all-directions.
> >
> > Since we didn't noticed that the "test ! -f" harms so it has been
> > there but finally we need to remove it.  Instead, we need to write
> > doen the known significant requirements by words. I'm afraid that the
> > concrete script would be a bit complex for the documentation..
>
> We don't have any 'officially-provided' tool for archive command.

> > The replacement would be something like:
> >
> > "There is a case where WAL file and timeline history files is archived
> > more than once.  The archive command should generally be designed to
> > refuse to replace any pre-existing archive file with a file with
> > different content but to return zero if the file to be archived is
> > identical with the preexisting file."
> >
> > But I'm not sure how it looks like.. (even ignoring the broken
> > phrasing..)
>
> There is so much more that we should be including here, like "you should

> make sure your archive command will reliably sync the WAL file to disk
> before returning success to PG, since PG will feel free to immediately
> remove the WAL file once archive command has returned successfully", and
> "the archive command should check that there exists a .history file for
> any timeline after timeline 1 in the repo for the WAL file that's being
> archived" and "the archive command should allow the exist, binary
> identical, WAL file to be archived multiple times without error, but
> should error if a new WAL file is archived which would overwrite a
> binary distinct WAL file in the repo", and "the archive command should
> check the WAL header to make sure that the WAL file matches the cluster
> in the corresponding backup repo", and "whatever is expiring the WAL
> files after they've been archived should make sure to not expire out any
> WAL that is needed for any of the backups that remain", and "oh, by the
> way, depending on the exit code of the command, PG may consider the
> failure to be something which can be retried, or not", and other things
> that I can't think of off the top of my head right now.
> I have to say that it gets to a point where it feels like we're trying
> to document everything about writing a C extension to PG using the
> hooks which we make available.  We've generally agreed that folks should
> be looking at the source code if they're writing a serious C extension
> and it's certainly the case that, in writing a serious archive command
> and backup tool, getting into the PG source code has been routinely
> necessary.

>
> Nevertheless I agree to it, still don't we need a minimum workable
> setup as the first step? Something like below.
>
> ===
> The following is an example of the minimal archive_command.
>
> Example: cp %p /blah/%f
>
> However, it is far from perfect. The following is the discussion about
> what is needed for archive_command to be more reliable.
>
>
> ====

> On Fri, Jun 11, 2021 at 11:25:51AM +0900, Kyotaro Horiguchi wrote:
> >
> > Nevertheless I agree to it, still don't we need a minimum workable
> > setup as the first step? Something like below.
> >
> > ===
> > The following is an example of the minimal archive_command.
> >
> > Example: cp %p /blah/%f
> >
> > However, it is far from perfect. The following is the discussion about
> > what is needed for archive_command to be more reliable.
> >
> >
> > ====
>
> "far from perfect" is a strong understatement for "appears to work but will
> randomly and silently breaks everything without a simple way to detect it".

> > We should document a minimum workable setup, but cp isn't an example of that,
> > and I don't think that there will be a simple example unless we provide a
> > dedicated utility.
>
> It looks somewhat strange like "Well, you need a special track to
> drive your car, however, we don't have one. It's your responsibility
> to construct a track that protects it from accidents perfectly.".
>
> "Yeah, I'm not going to push it so hard and don't care it gets some
> small scratches, couldn't I drive it on a public road?"
>
> (Sorry for the bad analogy).