Re: Patch: show relation and tuple infos of a lock to acquire

On Thu, Mar 13, 2014 at 12:45 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> While attempting to "operate in"?  That seems like unhelpful
>> weasel-wording.  I wonder if we ought to have separate messages for
>> each possibility, like "delete tuple (X,Y)" when called from
>> heap_delete(), "update tuple (X,Y)", "check exclusion constraint on
>> tuple (X,Y)" when called from check_exclusion_constraint, etc.  That
>> seems like it would be handy information to have.
>
> Okay, below are the distinct places from where we need to pass
> such information.
>
> heap_delete - "delete tuple (X,Y)"
> heap_update - "update tuple (X,Y)"
> heap_lock_tuple - "lock tuple (X,Y)"
> heap_lock_updated_tuple_rec - "lock updated tuple (X,Y)"
> _bt_doinsert - "insert index tuple (X,Y)" (here it will refer to index tuple
> location)

I don't think that giving the index tuple location is going to be very
helpful; can we get the TID for conflicting heap tuple?

> IndexBuildHeapScan - "scan tuple (X,Y)"
> EvalPlanQualFetch - "fetch tuple (X,Y)"

These two seem unhelpful to me.  For EvalPlanQualFetch maybe "recheck
updated tuple" would be good, and for IndexBuildHeapScan perhaps
"checking uniqueness of tuple".

> check_exclusion_constraint - "check exclusion constraint on tuple (X,Y)"
>
> I think it might not be a big deal to update the patch to pass such info.
> Won't it effect the translatability guidelines as we need to have different
> translation message for each op?

Yes, we'll need a separate message for each.

> The other option could be we need to ensure which places are safe to
> pass tuple so that we can display whole tuple instead of just TID,
> for example the tuple we are passing from heap_lock_tuple() has been
> fetched using Dirty Snapshot (refer EvalPlanQualFetch() caller of
> heap_lock_tuple()), but still we can use it in error as it has been decided
> that it is live tuple and transaction can update it by the time it reaches
> XactLockTableWaitWithInfo(), so it is safe. I think we need to discuss
> and validate all places where ever we use Dirty/Any Snapshot to ensure
> that we can pass tuple from such a call, may be at end the result is
> we can pass tuple from most of locations, but still it needs to be done
> carefully.

Well, it's sounding like we can only display the whole tuple if (1)
the message level is less than ERROR and (2) the snapshot is an MVCC
snapshot.  That's an annoying and hard-to-document set of limitations.But we should be able to display the TID always,
soI think we should
 
drop the part of the patch that tries to show tuple data and revisit
that in a future release if need be.

I don't feel too bad about that because it seems to me that showing
the TID is a big improvement over the status quo; right now, when you
get the information that transaction A is waiting for transaction B,
you know they're fighting over some tuple, but you have no idea which
one.  Even just having the relation name would help a lot, I bet, but
if you have the TID also, you can use a SELECT query with WHERE ctid =
'(X,Y)' to find the specific tuple of interest.  That's maybe not as
convenient as having all the data printed out in the log, and there
are certainly use cases it won't answer, but it's still a WHOLE lot
better than having absolutely NO information, which is what we've got
today.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company