Re: lock_timeout GUC patch

From: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Hans-Juergen Schoenig <postgres(at)cybertec(dot)at>, Josh Berkus <josh(at)agliodbs(dot)com>, Sándor Miglécz <sandor(at)cybertec(dot)at>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: lock_timeout GUC patch
Date: 2010-01-13 19:12:33
Message-ID: 4B4E1B21.6070907@cybertec.at
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane írta:
> Boszormenyi Zoltan <zb(at)cybertec(dot)at> writes:
>
>> Tom Lane írta:
>>
>>> If this patch is touching those parts of relcache.c, it probably needs
>>> rethinking.
>>>
>
>
>> What I did there is to check the return value of LockRelationOid()
>> and also elog(PANIC) if the lock wasn't available.
>> Does it need rethinking?
>>
>
> Yes. What you have done is to change all the LockSomething primitives
> from return void to return bool and thereby require all call sites to
> check their results. This is a bad idea.

Okay, can you tell me how can I get the relation name
out of the xid in XactLockTableWait()? There are several
call site of this function, and your idea about putting the error
code into the LockSomething() functions to preserve the API
results strange error messages, like

ERROR: could not obtain lock on transaction with ID 658

when I want to UPDATE a tuple in a session when
this and another session have a FOR SHARE lock
on said tuple.

> There is no way that you can
> ensure that all third-party modules will make the same change, meaning
> that accepting this patch will certainly introduce nasty, hard to
> reproduce bugs. And what's the advantage? The callers are all going
> to throw errors anyway, so you might as well do that within the Lock
> function and avoid the system-wide API change.
>
> I think this is a big patch with a small patch struggling to get out.
>

Your smaller patch is attached, with the above strangeness. :-)

Best regards,
Zoltán Böszörményi

--
Bible has answers for everything. Proof:
"But let your communication be, Yea, yea; Nay, nay: for whatsoever is more
than these cometh of evil." (Matthew 5:37) - basics of digital technology.
"May your kingdom come" - superficial description of plate tectonics

----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/

Attachment Content-Type Size
5-pg85-locktimeout-12-ctxdiff.patch text/x-patch 32.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-01-13 19:15:09 Re: plpython3
Previous Message Josh Berkus 2010-01-13 19:05:20 Re: TPR-tree crash WAS: NEED HELP !