Re: SELECT ... FOR UPDATE [WAIT integer | NOWAIT] for 8.5

Jeff Janes írta:
> On Thu, Sep 3, 2009 at 6:47 AM, Boszormenyi Zoltan <zb(at)cybertec(dot)at
> <mailto:zb(at)cybertec(dot)at>> wrote:
>
> Boszormenyi Zoltan írta:
> > Alvaro Herrera írta:
> >
> >> Boszormenyi Zoltan wrote:
> >>
> >>
> >>
> >>> The vague consensus for syntax options was that the GUC
> >>> 'lock_timeout' and WAIT [N] extension (wherever NOWAIT
> >>> is allowed) both should be implemented.
> >>>
> >>> Behaviour would be that N seconds timeout should be
> >>> applied to every lock that the statement would take.
> >>>
> >>>
> >> In
> http://archives.postgresql.org/message-id/291.1242053201@sss.pgh.pa.us
> >> Tom argues that lock_timeout should be sufficient. I'm not
> sure what
> >> does WAIT [N] buy
>
>
> I disagree with Tom on this point. *If* I was trying to implement a
> server policy, then sure, it should not be done by embedding the
> timeout in the SQL statement. But I don't think they want this to
> implement a server policy. (And if we do, why would we thump the poor
> victims that are waiting on the lock, rather than the rogue who
> decided to take a lock and then camp out on it?) The use case for
> WAIT [N] is not a server policy, but a UI policy. I have two ways to
> do this task. The preferred way needs to lock a row, but waiting for
> it may take too long. So if I can't get the lock within a reasonable
> time, I fall back on a less-preferred but still acceptable way of
> doing the task, one that doesn't need the lock. If we move to a new
> server, the appropriate value for the time out does not change,
> because the appropriate level is the concern of the UI and the end
> users, not the database server. This wouldn't be scattered all over
> the application, either. In my experience, if you have an application
> that could benefit from this, you might have 1 or 2 uses for WAIT [N]
> out of 1,000+ statements in the application. (From my perspective, if
> there were to be a WAIT [N] option, it could plug into the
> statement_timeout mechanism rather than the proposed lock_timeout
> mechanism.)
>
> I think that if the use case for a GUC is to set it, run a single very
> specific statement, and then unset it, that is pretty clear evidence
> that this should not be a GUC in the first place.
>
> Maybe I am biased in this because I am primarily thinking about how I
> would use such a feature, rather than how Hans-Juergen intends to use
> it, and maybe those uses differ. Hans-Juergen, could you describe
> your use case a little bit more? Who do is going to be getting these
> time-out errors, the queries run by the web-app, or longer running
> back-office queries? And when they do get an error, what will they do
> about it?

Our use case is to port a huge set of Informix apps,
that use SET LOCK MODE TO WAIT N;
Apparently Tom Lane was on the opinion that
PostgreSQL won't need anything more in that regard.

In case the app gets an error, the query (transaction)
can be retried, the "when" can be user controlled.

I tried to argue on the SELECT ... WAIT N part as well,
but for our purposes currently the GUC is enough.

> > Okay, we implemented only the lock_timeout GUC.
> > Patch attached, hopefully in an acceptable form.
> > Documentation included in the patch, lock_timeout
> > works the same way as statement_timeout, takes
> > value in milliseconds and 0 disables the timeout.
> >
> > Best regards,
> > Zoltán Böszörményi
> >
>
> New patch attached. It's only regenerated for current CVS
> so it should apply cleanly.
>
>
>
> In addition to the previously mentioned seg-fault issues when
> attempting to use this feature (confirmed in another machine, linux,
> 64 bit, and --enable-cassert does not offer any help), I have some
> more concerns about the patch. From the docs:
>
> doc/src/sgml/config.sgml
>
> Abort any statement that tries to lock any rows or tables and
> the lock
> has to wait more than the specified number of milliseconds,
> starting
> from the time the command arrives at the server from the client.
> If <varname>log_min_error_statement</> is set to
> <literal>ERROR</> or
> lower, the statement that timed out will also be logged.
> A value of zero (the default) turns off the limitation.
>
> This suggests that all row locks will have this behavior. However, my
> experiments show that row locks attempted to be taken for ordinary
> UPDATE commands do not time out. If this is only intended to apply to
> SELECT .... FOR UPDATE, that should be documented here. It is
> documented elsewhere that this applies to SELECT...FOR UPDATE, but it
> is not documented that this the only row-locks it applies to.
>
> "from the time the command arrives at the server". I am pretty sure
> this is not the desired behavior, otherwise how does it differ from
> statement_timeout? I think it must be a copy and paste error for the doc.
>
>
> For the implementation, I think the patch touches too much code. In
> particular, lwlock.c. Is the time spent waiting on ProcArrayLock
> significant enough that it needs all of that code to support timing it
> out? I don't think it should ever take more than a few microseconds
> to obtain that light-weight lock. And if we do want to time all of
> the light weight access, shouldn't those times be summed up, rather
> than timing out only if any single one of them exceeds the threshold
> in isolation? (That is my interpretation of how the code works
> currently, I could be wrong on that.)

You seem to be right, it may not be needed.
The only callsite is ProcSleep() in storage/lmgr/proc.c
and PGSemaphoreTimedLock() was already waited on.
Thanks for the review.

>
> If the seg-faults are fixed, I am still skeptical that this patch is
> acceptable, because the problem it solves seems to be poorly or
> incompletely specified.
>
> Cheers,
>
> Jeff

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