Re: Strange Windows problem, lock_timeout test request - Mailing list pgsql-hackers

* Boszormenyi Zoltan (zb@cybertec.at) wrote:
> 2013-02-24 03:23 keltezéssel, Stephen Frost írta:
> >No, it isn't.  Patches posted to the list should be in context diff
> >format (and uncompressed unless it's too large) for easier reading.
> >That avoids having to download it, apply it to a git tree and then
> >create the diff ourselves.  Indeed, the move to git had impact on this
> >at all.
>
> I remembered this mail from The Master(tm):
> http://www.postgresql.org/message-id/21555.1339866293@sss.pgh.pa.us

Which matches exactly what I was saying- context diff provides easier
readind for review purposes, which is presumably what you were looking
to have happen when you posted this patch to the mailing list.  And it's
still the documented expectation and project policy, regardless of any
individual's email.  If we're going to change it, then the
documentation, et al, should be updated as well.

For my 2c, I still prefer context diffs posted to the mailing lists,
but would also like to see more people posting pull requests.  That
doesn't make it project policy though.

> So, more than halfof the recently posted patches come
> directly from "git diff". The preference has changed.

No, more people have ignored the project policy than not- that doesn't
say anything about the preference or what the policy is.

> >lock_timeout_wait and lock_timeout_stmt_wait ?
>
> Hm. How about without the "_wait" suffix?
> Or lock_timeout vs statement_lock_timeout?

I could live without the _wait suffix, but it occurs to me that none of
these really indicate that statement_lock_timeout is an accumulated
timeout.  Perhaps it should say 'total lock wait timeout' or similar?

> statement_timeout is the legacy behaviour, it should be kept.
> It's behaviour is well understood: the statement finishes under
> the specified time or it throws an error. The problem from the
> application point of view is that the error can happen while
> the tuples are being transferred to the client, so the recordset
> can be cut in half.
>
> "lock_timeout_stmt" (or lock_timeout_option = per_statement)
> is somewhat extending the statement_timeout as only the
> time waiting on locks are counted, so SELECT FOR UPDATE/etc.
> may throw an error but if all rows are collected already, or
> plain SELECT is run, the application gets them all.
> This seems to follow the Microsoft SQL Server semantics:
> http://msdn.microsoft.com/en-us/library/ms189470.aspx

The documentation at that link appears to match what 'lock_timeout'
would do.  Note that it says 'a lock' here: "When a wait for a lock
exceeds the time-out value, an error is returned.", or have you tested
the actual behavior and seen it treat this value as an accumulated
time across all lock waits for an entire statement?

> The per-lock lock_timeout was the result of a big Informix
> project ported to PostgreSQL, this follows Informix semantics:
> http://publib.boulder.ibm.com/infocenter/idshelp/v10/index.jsp?topic=/com.ibm.sqls.doc/sqls788.htm

I agree that the Informix command looks to be per-lock, but based on my
simple reading of both documentation links, I think they're actually the
same behavior and neither is about an accumulated time.

> Because Timestamp[Tz] is microsecond resolution, and the timeout
> GUCs are in milliseconds. Adding up time differences (and rounding
> or truncating them to millisecond in the process) would make the
> per-statement lock timeout lose precision...

Removing the accumulation-based option would fix that.. :D
Thanks,
    Stephen