Re: commit dfda6ebaec67 versus wal_keep_segments

From: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: commit dfda6ebaec67 versus wal_keep_segments
Date: 2013-04-03 19:50:57
Message-ID: CAMkU=1wNGwzu-Cu-bYmYSqhS0kx92CTY4XeqrMt3orcQHb=AWw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 3, 2013 at 11:14 AM, Heikki Linnakangas <hlinnakangas(at)vmware(dot)com
> wrote:

> On 03.04.2013 18:58, Jeff Janes wrote:
>
>> On Tue, Apr 2, 2013 at 10:08 PM, Jeff Janes<jeff(dot)janes(at)gmail(dot)com> wrote:
>>
>> This commit introduced a problem with wal_keep_segments:
>>>
>>> commit dfda6ebaec6763090fb78b458a979b**558c50b39b
>>>
>>
>> The problem seems to be that the underflow warned about is happening,
>> because the check to guard it was checking the wrong thing. However, I
>> don't really understand KeepLogSeg. It seems like segno, and hence
>> recptr,
>> don't actually serve any purpose.
>>
>
> Hmm, the check is actually correct, but the assignment in the else-branch
> isn't. The idea of KeepLogSeg is to calculate recptr - wal_keep_segments,
> and assign that to *logSegNo. But only if *logSegNo is not already < than
> the calculated value. Does the attached look correct to you?

Let me describe what I think is going on. My description is "On start,
recptr is the redo location of the just-completed checkpoint, and logSegNo
is the redo location segment of the checkpoint before that one. We want to
keep the previous-checkpoint redo location, and we also want to keep
wal_keep_segments before the current-checkpoint redo location, so we take
whichever is earlier."

If my understanding is now correct, then I think your patch looks correct.
(Also, applying it fixed the problem I was having.)

Why do we keep wal_keep_segments before the just-finished checkpoint,
rather than keeping that many before the previous checkpoint? I seems like
it would be more intuitive (to the DBA) for that parameter to mean "keep
this many more segments than you otherwise would". I'm not proposing we
change it, I'm just curious about why it is done that way.

Thanks,

Jeff

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2013-04-03 20:45:02 Re: Drastic performance loss in assert-enabled build in HEAD
Previous Message Andres Freund 2013-04-03 19:33:05 Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)