Re: lock_timeout GUC patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Boszormenyi Zoltan <zb(at)cybertec(dot)at>
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 14:56:58
Message-ID: 10386.1263394618@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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. 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.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-01-13 15:15:14 Re: Bloom index
Previous Message Jaime Casanova 2010-01-13 14:39:29 Re: lock_timeout GUC patch