Hi, On 2017-11-14 01:30:30 +1300, Thomas Munro wrote: > New patch attached.
0002-Add-a-barrier-primitive-for-synchronizing-backends.patch - Intro starts with: + * + * This implementation of barriers allows for static sets of participants + * known up front, or dynamic sets of participants which processes can join or that's a bit low-level, no? Should explain what they're for, not everyone's going to be familiar with the concept. - The comment for BarrierInit() reads a bit weird: + * Initialize this barrier, setting a static number of participants that we + * will wait for at each computation phase. To use a dynamic number of + * participants, this number should be zero, and BarrierAttach and Set a static number! Except maybe not? - I'm not entirely convinced that we want the barrier debug stuff merged, what are your feelings about that? It's like half the code, and adds some complexity to the non debug code... If we indeed want to keep it, it probably should be documented in config.sgml? And get an entry in pg_config_manual.h? - Can we add assertions ensuring nobody attaches to a static barrier? - If I understand correctly currently the first participant to arrive at a barrier is going to be selected, and the last wakes everyone up. Wouldn't it be better to do have the last arrived participant be selected? It already got a scheduler timeslice, it's memory is most likely to be in cache etc? Given that in cases where selection plays a role it's going to be blocking everyone else, using the process most likely to finish first seems like a good idea. I guess the BarrierDetach() implementation would be slightly more complex? Regards, Andres
