Обсуждение: trivial patch: show SIREAD pids in pg_locks
While looking into a SSI bug, I noticed that we don't actually display the pid of the holding transaction, even though we have that information available. The attached patch fixes that. One note is that it will show the pid of the backend that executed the transaction, even if that transaction has already committed. I have no particular opinion about whether it's more useful to do that or return null, so went with the smallest change. (The pid is null for PREPARED or summarized transactions). Dan -- Dan R. K. Ports MIT CSAIL http://drkp.net/
Вложения
Dan Ports <drkp@csail.mit.edu> wrote: > While looking into a SSI bug, I noticed that we don't actually > display the pid of the holding transaction, even though we have > that information available. I thought we already had that, but clearly I was mistaken. > The attached patch fixes that. > > One note is that it will show the pid of the backend that executed > the transaction, even if that transaction has already committed. I > have no particular opinion about whether it's more useful to do > that or return null, so went with the smallest change. (The pid is > null for PREPARED or summarized transactions). The patch looks good to me and a quick test shows the expected behavior. No warnings. Regression tests pass. I guess the question is whether it's OK to include this during the alpha testing phase. Even though it's a little bit of a stretch to call it a bug, the argument could be made that omitting information which all the other rows in the view have is an inconsistency which borders on being a bug. The small size and verifiable safety of the patch work in its favor. -Kevin
On Fri, Apr 01, 2011 at 12:20:25PM -0500, Kevin Grittner wrote: > I thought we already had that, but clearly I was mistaken. Yeah, so did I. Turns out we had the vxid but not the pid. IIRC, we weren't tracking a SERIALIZABLEXACT's pid yet, at the time we wrote the code for pg_locks. > I guess the question is whether it's OK to include this during the > alpha testing phase. Even though it's a little bit of a stretch to > call it a bug, the argument could be made that omitting information > which all the other rows in the view have is an inconsistency which > borders on being a bug. The small size and verifiable safety of the > patch work in its favor. There's no urgent need to have this, although it's obviously more correct than the current behavior. It might be useful for debugging. The patch is also all of four lines. :-) Dan -- Dan R. K. Ports MIT CSAIL http://drkp.net/
Dan Ports <drkp@csail.mit.edu> wrote: > There's no urgent need to have this, although it's obviously more > correct than the current behavior. It might be useful for > debugging. Agreed all around. For the benefit of the more casual follower of the thread, attached is a simple example of the output. For the SIReadLock rows, the pid column shows as empty without the patch. -Kevin
Вложения
On Fri, 2011-04-01 at 13:00 -0400, Dan Ports wrote: > While looking into a SSI bug, I noticed that we don't actually display > the pid of the holding transaction, even though we have that > information available. Is there a chance that the PID will reference a backend that has either terminated or is idle? That might be confusing. Regards,Jeff Davis
Jeff Davis <pgsql@j-davis.com> wrote: > On Fri, 2011-04-01 at 13:00 -0400, Dan Ports wrote: >> While looking into a SSI bug, I noticed that we don't actually >> display the pid of the holding transaction, even though we have >> that information available. > > Is there a chance that the PID will reference a backend that has > either terminated or is idle? Yes to both. > That might be confusing. With a barely larger patch we could suppress that, if that's desirable. Of course, there is already the same issue for the virtualtransaction column. As Dan mentioned, it won't show for locks which are summarized using SLRU, nor will it show for prepared transactions pending final commit -- at least if they're loaded from disk after recovery. (I'm not sure without some digging about a transaction which is prepared and still pending commit if the server is still running from the point of prepare.) -Kevin
On Fri, Apr 1, 2011 at 1:00 PM, Dan Ports <drkp@csail.mit.edu> wrote: > While looking into a SSI bug, I noticed that we don't actually display > the pid of the holding transaction, even though we have that > information available. > > The attached patch fixes that. Committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company