On 16 January 2013 16:12, Stephen Frost <sfr...@snowman.net> wrote: > * Simon Riggs (si...@2ndquadrant.com) wrote: >> > A couple quick notes regarding the patch- what does >> > GetXactLockTableRelid really provide..? >> >> The ability to access a static variable in a different module. It >> doesn't provide anything other than that, > > It isn't actually necessary for that currently though, is it? All calls > to it appeared to be inside the same module. Do you anticipate that > needing to change in the future?
OK, will change. >> The information isn't available, which is why I didn't include it. >> Comments explain that the additional information is only available >> within the backend that was waiting. That's sufficient for stats, but >> not enough for an extended multi-backend deadlock report. > > Right, sorry, to be clear, it seemed that we could simply detect if the > information is available and print it out if it is- and not print it if > it isn't. The additional boolean variable looked to be unnecessary, or > is there a point where OidIsValid(GetXactLockTableRelid()) will be true > but the additional information shouldn't be printed or isn't actually > available? You might be reporting a deadlock in another backend when you are waiting on a transaction yourself. If we don't address that we'll end up reporting the wrong info in a way that makes it look real and probably never even notice. >> There is an API change to XactLockTableWait() to pass the relid. That >> part is essential to any solution. > > I'm wondering if we could pass in something both more generic and with > more information- perhaps a simple string which is constructed when we > have more information and is then included in the log message which is > generated? Perhaps something which includes both the relname and the > OID? Could that be used across more than just these kinds of locks? Of > course, on the flip side, I understand that we don't want to spend a lot > of time building strings and whatnot during all of these code paths; > would be nice if we could defer that until we're about to log. Keeping it as an Oid is better, since we don't always need the string. > Regardless, I do think it'd be good to update the comments to indicate > that the relid is being passed in solely for the purpose of being > available to be included in the log, if necessary. OK, will add. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
