On Wed, Jan 10, 2018 at 5:05 PM, Peter Geoghegan <p...@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