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
Subject: Re: lock_timeout GUC patch
Date: 2010-01-20 09:29:59
Message-ID: 4B56CD17.6010109@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:
>
>> [ 5-pg85-locktimeout-14-ctxdiff.patch ]
>>
>
> I took a quick look at this. I am not qualified to review the Win32
> implementation of PGSemaphoreTimedLock, but I am afraid that both of
> the other ones are nonstarters on portability grounds. sem_timedwait()
> and semtimedop() do not appear in the Single Unix Spec, which is our
> usual reference for what is portable. In particular I don't see either
> of them on OS X or HPUX.

We're lucky in that regard, we have developed and tested this patch
under Linux and:

# uname -a
HP-UX uxhv1f17 B.11.31 U ia64 4099171317 unlimited-user license

The links under src/backend/port show that it uses sysv_sema.c
and semtimedop() compiles and works nicely there.

Hans will test it under OS X.

> I suspect that applying this patch would
> immediately break every platform except Linux.
>

Fortunately suspicion doesn not mean guilty, let's wait for Hans' test.

> I also concur with Alvaro's feeling that the changes to XactLockTableWait()
> and MultiXactIdWait() are inappropriate. There is no reason to assume
> that there is always a relevant relation for waits performed with those
> functions. (In the same line, not all of the added error reports are
> careful about what happens if get_rel_name fails.)
>

Okay, I don't have strong feelings about the exact error message,
I will post the older version with the Lock* APIs intact, add the chunk
that adds the GUC to postgresql.conf.sample and also look at your
comment. But IIRC some of the missing checks come from the callers'
logic, they (all or only some of them? have to check) already opened
the Relation they try to lock hence the same get_rel_name() MUST
succeed or else it's an internal error already.

> A larger question, which I think has been raised before but I have not
> seen a satisfactory answer for, is whether the system will behave sanely
> at all with this type of patch in place. I don't really think that a
> single lock timeout applicable to every possible reason to wait is going
> to be nice to use;

IIRC you were the one who raised the issue but in the exact
opposite way to conclude that we won't need SELECT ... WAIT N
to complement NOWAIT. Stick to one opinion please. :-)

> and I'm afraid in some contexts it could render
> things completely nonfunctional. (In particular I think that Hot
> Standby is fragile enough already without this.) It seems particularly
> imprudent to make such a thing USERSET, implying that any clueless or
> malicious user could set it in a way that would cause problems, if there
> are any to cause.
>

Is there an flag that causes the setting rejected from postgresql.conf
but makes settable from the session? This would ensure correct operation,
as the default 0 behaves the same as before.

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/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2010-01-20 09:35:10 Re: An example of bugs for Hot Standby
Previous Message Heikki Linnakangas 2010-01-20 09:27:03 Re: Git out of sync vs. CVS