On Tue, Oct 4, 2016 at 3:12 PM, Thomas Munro <thomas.mu...@enterprisedb.com> wrote: > On Mon, Aug 15, 2016 at 5:58 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> Please find attached a -v2 of your patch which includes suggestions >> 1-3 above. > > Here's a rebased patch. ConditionVariableSleep now takes > wait_event_info. Anyone using this in patches for core probably needs > to add enumerators to the WaitEventXXX enums in pgstat.h to describe > their wait points.
So, in my original patch, it was intended that cvSleeping was the definitive source of truth as to whether a particular backend is committed to sleeping or not. That had some bugs, I guess, but in your version, there are now two sources of truth. On the one hand, there's still cvSleeping. On the other hand, there's now also whether proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink) returns true. I'm not sure whether that causes any serious problem. It seems possible, for example, that ConditionVariableSignal() could clear the cvSleeping flag for a backend that's now waiting for some OTHER condition variable, because once it pops the victim off the list and releases the spinlock, the other backend could meanwhile ConditionVariableCancelSleep() and then do anything it likes, including sleep on some other condition variable. Now I think that the worst thing that will happen is that the other backend will receive a spurious wakeup, but I'm not quite sure. The whole point of having cvSleeping in the first place is so that we don't get spurious wakeups just because somebody happens to set your process latch, so it seems a bit unfortunate that it doesn't actually prevent that from happening. I wonder if we shouldn't try to create the invariant that when the CV mutex is not help, the state of cvSleeping has to be true if we're in the proclist and false if we're not. So ConditionVariableSignal() would clear the flag before releasing the spinlock, for example. Then I think we wouldn't need proclist_contains(). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers