On Wed, Jan 10, 2018 at 5:05 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> How about I remove the comment, but have tuplesort_begin_common()
> force each Tuplesortstate to have workMem that is at least 64KB
> (minimum legal work_mem value) in all cases? We can just formalize the
> existing assumption that workMem cannot go below 64KB, really, and it
> isn't reasonably to use so little workMem within a parallel worker (it
> should be prevented by plan_create_index_workers() in the real world,
> where parallelism is never artificially forced).
+1. I think this doesn't even need to be documented. You can simply
write a comment that says something /* Always allow each worker to use
at least 64kB. If the amount of memory allowed for the sort is very
small, this might technically cause us to exceed it, but since it's
tiny compared to the overall memory cost of running a worker in the
first place, it shouldn't matter. */
> I share your general feelings on all of this, but I really don't know
> what to do about it. Which of these alternatives is the least worst,
> all things considered?
Let's get the patch committed without any explicit way of forcing the
number of workers and then think about adding that later.
It will be good if you and Rushabh can agree on who will produce the
next version of this patch, and also if I have some idea when that
version should be expected. On another point, we will need to agree
on how this should be credited in an eventual commit message. I do
not agree with adding Heikki as an author unless he contributed code,
but we can credit him in some other way, like "Thanks are also due to
Heikki Linnakangas for significant improvements to X, Y, and Z that
made this patch possible." I assume the author credit will be "Peter
Geoghegan, Rushabh Lathia" in that order, but let me know if anyone
thinks that isn't the right idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
