At Thu, 21 Apr 2016 14:05:28 -0400, Robert Haas <robertmh...@gmail.com> wrote in <ca+tgmobyx6+d695se3-uz1hl326at-9sr4jzqxbw3gaw4ex...@mail.gmail.com> > >>> Here is PoC patch which fixes the problem. I am wondering if we should > >>> raise warning in the pg_stat_get_backend_wait_event_type() and > >>> pg_stat_get_backend_wait_event() like the pg_signal_backend() does > >>> when proc is NULL instead of just returning NULL which is what this > >>> patch does though. > >> > >> It still makes the two relevant columns in pg_stat_activity > >> inconsistent each other since it reads the procarray entry twice > >> without a lock on procarray. > >> > >> The attached patch adds pgstat_get_wait_event_info to read > >> wait_event_info in more appropriate way. Then change > >> pg_stat_get_wait_event(_type) to take the wait_event_info. > >> > >> Does this work for you? > > > > This is a hideously way of fixing this problem. The whole point of > > storing the wait event in a 4-byte integer is that we already assume > > reads of 4 byte integers are atomic and thus no lock is needed.
A lock was already held in BackendPidGetProc(). Is it also needless? If so, we should use BackendPidGetProcWithLock() instad (the name seems a bit confusing, though). What I did in the patch was just extending the lock duration until reading the pointer proc. I didn't added any additional lock. > > The > > only thing we need to do is to prevent the value from being read > > twice, and we already have precedent for how to prevent that in > > freelist.c. However, I don't have objections for the patch applied. > That was intended to say "a hideously expensive way". Thanks for the kind suppliment.. -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
