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/
Attachments:
patch-ssi-pid-in-pg_locks.difftext/x-diff; charset=us-asciiDownload
diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index c6c948c..6d7d4f4 100644
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -368,7 +368,10 @@ pg_lock_status(PG_FUNCTION_ARGS)
/* lock holder */
values[10] = VXIDGetDatum(xact->vxid.backendId,
xact->vxid.localTransactionId);
- nulls[11] = true; /* pid */
+ if (xact->pid != 0)
+ values[11] = Int32GetDatum(xact->pid);
+ else
+ nulls[11] = true;
/*
* Lock mode. Currently all predicate locks are SIReadLocks, which are
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
Attachments:
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