Re: Patch to implement pg_current_logfile() function

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
Cc: Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>, Christoph Berg <myon(at)debian(dot)org>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to implement pg_current_logfile() function
Date: 2016-12-13 15:26:52
Message-ID: CA+TgmoZpzGS8hW3pFw8-_c5eoczb4t5pAaeetpf7NzY+FnwdRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Dec 8, 2016 at 4:34 PM, Karl O. Pinc <kop(at)meme(dot)com> wrote:
>> I do think that the blizzard of small patches that you've
>> submitted in some of your emails may possibly be a bit overwhelming.
>> We shouldn't really need a stack of a dozen or more patches to
>> implement one small feature. Declarative partitioning only had 7.
>> Why does this need more than twice that number?
>
> I'm trying to break up changes into patches which are as small
> as possible to make them more easily understood by providing
> a guide for the reader/reviewer. So rather than
> a single patch I'd make, say, 3. One for documentation to describe
> the change. Another which does whatever refactoring is necessary
> to the existing code, but which otherwise does not introduce any
> functional changes. And another which adds the new code which makes
> use of the refactoring. At each stage the code should continue
> to work without bugs. The other party can then decide to incorporate
> the patchs into the main patch, which is itself another attached
> patch.

Sure, I don't think the intention is bad. It didn't really work out
here, perhaps because the individual changes were too small. I think
for small changes it's often more helpful to say "change X to be like
Y" than to provide a patch, or else it's worth combining several small
patches just to keep the number from getting too big. I don't know; I
can't point to anything you really did wrong here; the process just
didn't seem to be converging.

> It is worth noting that the PG pg_current_logfile() function
> is dependent upon the content of the current_logfiles file. So
> if the file is wrong the function will also give wrong answers.
> But I don't care if you don't.

Well, I want the current_logfiles file to be correct, but that doesn't
mean that it's reasonable to expect people to enumerate all the
logfiles that have ever existed by running it. That would be fragile
for tons of reasons, like whether or not the server hits the
connection limit at the wrong time, whether network connectivity is
unimpaired, whether the cron job that's supposed to run it
periodically hiccups for some stupid reason, and many others.

> A related thought is this. There are 3 abstraction levels of
> concern. The level beneath PG, the PG level, and the level
> of the user's application. When PG detects an error from either
> "above" or "below" its job is to describe the problem from
> its own perspective. HINTs are attempts to cross the boundary
> into the application's perspective.

Yes, I agree. EnterpriseDB occasionally get complaints from our
customers saying that PG is buggy because it reported an operating
system error. No, really, if the operating system can't read the
file, that's not our fault! Call your sysadmin! I also agree with
your perspective on HINTs.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-12-13 15:33:56 Re: pg_catversion builtin function
Previous Message Robert Haas 2016-12-13 15:19:35 Re: Patch to implement pg_current_logfile() function