Re: pg_archivecleanup bug

On 18 March 2014 14:15, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
> Bruce Momjian escribió:
>> On Tue, Mar 18, 2014 at 10:03:46AM -0400, Robert Haas wrote:
>> > On Tue, Mar 18, 2014 at 9:56 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> > > Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> > >> Very good point. I have modified the patch to add this block in all
>> > >> cases where it was missing. I started to wonder about the comment and
>> > >> if the Mingw fix was released. Based on some research, I see this as
>> > >> fixed in mingw-runtime-3.2, released 2003-10-10. That's pretty old.
>> > >
>> > > Yeah. I would vote for removing that code in all branches. There is no
>> > > reason to suppose somebody is going to install 8.4.22 on a machine that
>> > > they haven't updated mingw on since 2003. Or, if you prefer, just remove
>> > > it in HEAD --- but going around and *adding* more copies seems like
>> > > make-work. The fact that we've not heard complaints about the omissions
>> > > is good evidence that nobody's using the buggy mingw versions anymore.
>> >
>> > I don't think it is. Right now we're not checking errno *at all* in a
>> > bunch of these places, so we're sure not going to get complaints about
>> > doing it incorrectly in those places. Or do I need more caffeine?
>>
>> You are correct. This code is seriously broken and I am susprised we
>> have not gotten more complaints. Good thing readdir/closedir rarely
>> fail.

> back-patching

Some commentary on this...

Obviously, all errors are mine.

If pg_archivecleanup is a problem, then so is pg_standby a problem.

Given the above, this means we've run for about 7 years without a
reported issue on this. If we are going to "make this better" by
actually having it throw errors in places that didn't throw errors
before, are we sure that is going to make people happier? The archive
cleanup isn't exactly critical in most cases, so dynamic errors don't
matter much.

Also, the programs were originally written to work as standalone
program as well as an archive_cleanup_command. So we can't use
PostgreSQL infrastructure (can we?). That aspect is needed to allow
testing the program before it goes live.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services