From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Peter Geoghegan <pg(at)heroku(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Claudio Freire <klaussfreire(at)gmail(dot)com> |
Subject: | Re: Logical tape pause/resume |
Date: | 2016-12-21 13:25:49 |
Message-ID: | 108f4d21-13e2-af45-2bd7-6f32de640920@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 12/21/2016 03:22 PM, Robert Haas wrote:
> On Wed, Dec 21, 2016 at 7:25 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> For now, barring objections, I'm going to commit the first patch. It seems
>> like a worthwhile simplification in any case, especially for Peter's
>> Parallel tuplesort patch set.
>
> Well, it removes more code than it adds. That's definitely something.
> And saving memory per-empty-tape is good, too. A few random comments:
>
> "seeked" is kind of a lame variable name. How about "seekpos" or
> "newpos" or something like that?
Ok.
> /*
> * Even in minimum memory, use at least a MINORDER merge. On the other
> * hand, even when we have lots of memory, do not use more than a MAXORDER
> - * merge. Tapes are pretty cheap, but they're not entirely free. Each
> - * additional tape reduces the amount of memory available to build runs,
> - * which in turn can cause the same sort to need more runs, which makes
> - * merging slower even if it can still be done in a single pass. Also,
> - * high order merges are quite slow due to CPU cache effects; it can be
> - * faster to pay the I/O cost of a polyphase merge than to perform a single
> - * merge pass across many hundreds of tapes.
> + * merge. Tapes are pretty cheap, but they're not entirely free.
> High order
> + * merges are quite slow due to CPU cache effects; it can be faster to pay
> + * the I/O cost of a polyphase merge than to perform a single merge pass
> + * across many hundreds of tapes.
> */
>
> I think you could leave this comment as-is. You haven't zeroed out
> the overhead of a tape, and I like those additional bits I crammed in
> there. :-)
Yes, quite right. That was a mishap in rebasing, that change to remove
the comment really belongs to the pause/resume patch rather than the 1st
one.
Thanks for the review!
- Heikki
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2016-12-21 13:32:18 | Re: Replication slot xmin is not reset if HS feedback is turned off while standby is shut down |
Previous Message | Simon Riggs | 2016-12-21 13:23:41 | Re: Replication slot xmin is not reset if HS feedback is turned off while standby is shut down |