On 2014-01-30 12:27:43 -0500, Robert Haas wrote:
> Nope, but I think this patch is broken. It looks to me like it's
> conflating the process offset in the BackendStatus array with its
> backendId, which does not seem like a good idea even if it happens to
> work at present.
Hm. I don't see how that's going to be broken without major surgery in
pgstat.c. The whole thing seems to rely on being able to index
BackendStatusArray with MyBackendId?
> And the way BackendIdGetProc() is used looks unsafe,
> too: the contents might no longer be valid by the time we read them.
> I suspect we should have a new accessor function that takes a backend
> ID and copies the xid and xmin to pointers provided by the client
> while holding the lock. I also note that the docs seem to need some
> copy-editing:
It certainly needs to be documented as racy, but I don't see a big
problem with being racy here. We assume in lots of places that
writing/reading xids is atomic, and we don't even hold exclusive locks
while writing... (And yes, that means that the xid and xmin don't
necessarily belong to each other)
That said, encapsulating that racy access into a accessor function does
sound like a good plan.
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
