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
explainwhat 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
halfthe code, and adds some complexity to the non debug code... If we indeed want to keep it, it probably should be
documentedin 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
lastwakes everyone up. Wouldn't it be better to do have the last arrived participant be selected? It already got a
schedulertimeslice, it's memory is most likely to be in cache etc? Given that in cases where selection plays a role
it'sgoing to be blocking everyone else, using the process most likely to finish first seems like a good idea. I guess
theBarrierDetach() implementation would be slightly more complex?
Regards,
Andres
