On 16.06.2012 09:04, Amit kapila wrote:
>> I don't think so. C doesn't ref count its pointers.
> You are right I have misunderstood.
>
>> I don't think that lock tags have good human readable formats, and just
>> a pointer dump probably wouldn't be much use when something that can
>> never happen has happened. But I'll at least add a reference to the
>> resource owner if this stays in.
>
> I have checked in lock.c file for the message where lock tags have been used.
> elog(ERROR, "lock %s on object %u/%u/%u is already held",
> lockMethodTable->lockModeNames[lockmode],
> lock->tag.locktag_field1, lock->tag.locktag_field2,
> lock->tag.locktag_field3);
>
> This can give more information about erroneous lock.
A better error message would be nice, but I don't think it's worth that
much. resowner.c doesn't currently know about the internals of LOCALLOCk
struct or lock tags, and I'd prefer to keep it that way. Let's just
print the pointer's address, that's what we do in many other
corresponding error messages in other Forget* functions.
On 11.06.2012 18:21, Jeff Janes wrote:
> On Sun, Jun 10, 2012 at 11:28 PM, Amit Kapila<amit.kapila@huawei.com> wrote:
>> 2. ResourceOwnerForgetLock(), it decrements the lock count before removing,
>> so incase it doesn't find the lock in lockarray, the count will be
>> decremented inspite the array still contains the same number of locks.>
> Should it emit a FATAL rather than an ERROR? I thought ERROR was
> sufficient to make the backend quit, as it is not clear how it could
> meaningfully recover.
ERROR seems right to me, it'll get promoted to FATAL or PANIC if we're
in a context where we can't recover. But I agree with Amit that we
should not decrement the lock count on error. I'm not sure if it makes
any difference, as we'll abort out of the current transaction on error,
but it certainly looks wrong.
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com