On Thu, Apr 7, 2016 at 11:05 AM, Robert Haas <robertmh...@gmail.com> wrote: > I spent some time today reading through the new 0001 and in general I > think it looks pretty good.
Cool. > 1. Changing cost_sort to consider disk access as 90% sequential, 10% > random rather than 75% sequential, 25% random. As far as I can recall > from the thread, zero test results have been posted to demonstrate > that this is a good idea. It also seems completely unprincipled. I think that it's less unprincipled than the existing behavior, which imagines that I/O is a significant cost overall, something that is demonstrably wrong (there is an XXX comment about the existing disk access costings). Still, I agree that there is no logical reason to connect it to the bulk of what I want to do here, except that maybe it would be good if we were more optimistic about the cost of external sorting now. cost_sort() knows nothing about cache efficiency, of course, so naturally we cannot teach it to weigh cache efficiency less heavily. I guess I was worried that the smaller run sizes would put cost_sort() off external sorts even more, even as they became far cheaper. > 2. Restricting the maximum number of tapes to 500. This seems like a > sound change and I don't object to it in theory. But I've seen no > benchmark results which demonstrate that this is a good idea, and it > is quite separate from the core purpose of the patch. Ditto. This is something that could be done separately. We've often pondered if it made any sense at all (e.g. commit message of c65ab0bfa97b71bceae6402498910f4074996279), and I'm sure that it doesn't, but the memory refund stuff in the already memory management patch at least refunds the cost for the final on-the-fly merge (iff state->tuples). > Since time is short, I recommend we remove both of these things from > the patch and you can resubmit them as separate patches later. As far > as I can see, neither of them is so tied into the rest of the patch > that the main part of the patch can't be committed without those > changes. I agree to all this. Now that you've indicated where you stand on replacement_sort_mem, I have all the information I need to produce a new revision. I'll go do that. Thanks -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
