Re: Simplifying the interface of UpdateMinRecoveryPoint

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Simplifying the interface of UpdateMinRecoveryPoint
Date: 2016-08-26 09:53:21
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 07/13/2016 04:25 PM, Michael Paquier wrote:
> On Wed, Jul 13, 2016 at 8:27 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Wed, Jul 13, 2016 at 9:19 AM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>>> Hence why not simplifying its interface and remove the force flag?
>> One point to note is that the signature and usage of
>> UpdateMinRecoveryPoint() is same as it was when it got introduced in
>> commit-cdd46c76. Now the only reasons that come to my mind for
>> introducing the force parameter was (a) it looks cleaner that way to
>> committer (b) they have some usecase for the same in mind (c) it got
>> have overlooked. Now, if it got introduced due to (c), then your
>> patch does the right thing by removing it. Personally, I feel
>> overloading the parameter for multiple purposes makes code less
>> maintainable, so retaining as it is in HEAD has some merits.
> There is no way to tell what that is, but perhaps Heikki recalls
> something on the matter. I am just adding him in CC.

No, I don't remember. Maybe the function originally used the
caller-supplied 'lsn' value as the value to force-update
minRecoveryPoint to. Or I anticipated that some callers might want to do
that in the future.

If we were to do this, it might be better to still have a 'force'
variable inside the function, to keep the if()s slighltly more readable,

bool force = XLogRecPtrIsInvalid(lsn);

But even then, I don't think this makes it really any more readable
overall. Not worse either, but it's a wash. I'll just mark this as
rejected in the commitfest, let's move on.

- Heikki

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Christoph Berg 2016-08-26 10:44:46 Re: Renaming of pg_xlog and pg_clog
Previous Message Heikki Linnakangas 2016-08-26 09:31:22 Re: OpenSSL 1.1 breaks configure and more