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

2013-02-24 03:23 keltezéssel, Stephen Frost írta:
> Zoltán,
>
> * Boszormenyi Zoltan (zb@cybertec.at) wrote:
>> 2013-02-23 02:55 keltezéssel, Stephen Frost írta:
>>> First off, it's not in context diff format, which is the PG standard for
>>> patch submission.
>> Since moving to GIT, this expectation is obsolete.
> 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

>
>> All PG hackers
>> became comfortable with the unified diff format, see references
>> from the list. A lot of hackers post "git diff" patches which cannot
>> produce context diff, either.
> It's quite possible to create a context diff from git- indeed, it's
> documented on the http://wiki.postgresql.org/wiki/Working_with_Git
> portion of the wiki, specifically to help people understand how to take
> the changes they've made in their git forks and create context diffs to
> post to the mailing lists.  The preference for context diffs is also
> still documented at http://wiki.postgresql.org/wiki/Submitting_a_Patch.
>
> And, to be honest, I'm not sure how familiar you are with the history of
> PG in this area, but I've been through all of it from the pre-git CVS
> days, through the migration to git, to the exclusive use of git.

I started with Postgres95, though only using the source tarballs.
I started following CVS at around 2003 or so.

>    I feel
> quite confident that the preference for context diffs hasn't changed.

With stats for the February, 2013:
Name                 ctx  ctx/gitunif/git unif
Dimitry Fontaine 40       1      0
Ian Lawrence Barwick  0     1       0      0
Alvaro Herrera       10     0       5     0
Pavel Stehule         9     0       0      0
Heikki Linnakangas    0     0       7     0
Michael Paquier       0     0       6     0
Andres Freund         0     0       5     0
Alexander Law         0     0       1      0
Etsuro Fujita         0     0       4     0
Amit Kapila           4     0       0      0
Kevin Grittner        2     0       3      0
Gurjeet Singh         0     0       2    0
Erik Rijkers          0     0       0      1
Zoltan Boszormenyi    0     0       2      0
Tomas Vondra          0     0       2      0
Bruce Momjian         0     4       0      0
Fujii Masao           1     0       0      0
David Fetter          5     0       0      0
Jonathan Rogers       0     0       1      0
Andrew Dunstan        2     0       0      0
Manlio Perillo        0     0       1      0
Dean Rasheed          0     1       2      0
Jeff Janes            0     2       1      0
Phil Sorber           0     3       2      0
Mark Wong             0     1       0      0
Ivan Lezhnjov IV      0     0       1      0
Kohei Gagai           0     0       2      0
MauMau                1     0       0      0
Tom Lane              0     1       0      0
Sean Chittenden       0     0       2      0
Albe Laurenz          0     1       0      0
Daniel Farina         1     0       0      0
Simon Riggs           0     0       3      0
Peter Eisentraut      0     0       4      0

Plain context diffs: 39
Context diffs generated from git: 14
"git diff" patches: 57
Plain unified diff: 1

Some mails contained more than one patch, these were
counted as-is. One patch is one patch.

So, more than halfof the recently posted patches come
directly from "git diff". The preference has changed.
But indeed, plain "diff -u" is cleanly in the minority.
I can repost my patch from "git diff", too, to be in
the majority. :-P

>> That's not strictly true but it's not widely used either:
>>
>> $ find . -type f | xargs grep weight | grep heavy
>> ./monitoring.sgml:     <entry>Probe that fires when a request for a
>> heavyweight lock (lmgr lock)
>> ./monitoring.sgml:     <entry>Probe that fires when a request for a
>> heavyweight lock (lmgr lock)
> Interesting.  That didn't show up in the searches that I was doing
> through the web interface, though it does now.  If we're going to use
> that term, we should define it in the lock documentation.  If not, then
> we should avoid it everywhere.  I'm fine with either.

Me too, the documentation should be consistent. I will remove the
"heavyweight" term.

>
>>> - I don't particularly like "lock_timeout_option", for a couple of
>>>    reasons.  First is simply the name is terrible, but beyond that, it
>>>    strikes me that wanting to set both a 'per-lock timeout' and a
>>>    'overall waiting-for-locks timeout' at the same time would be a
>>>    reasonable use-case.  If we're going to have 2 GUCs and we're going to
>>>    support each of those options, why not just let the user specify
>>>    values for each?
>> OK, suggest names for the 2 GUCS, please.
> lock_timeout_wait and lock_timeout_stmt_wait ?

Hm. How about without the "_wait" suffix?
Or lock_timeout vs statement_lock_timeout?

>    Though I've been really
> wondering to myself as to if we need both of these options as well as
> statement_timeout.

>    Perhaps you can explain the use case for each
> option and how they're distinct from each other?

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

>    Indeed, the use-case
> that I'm envisioning is focused around "don't wait more than 'X' for the
> relation-level locks" which would allow you to distinguish queries that
> are, most likely anyway, making progress, from those which have been
> caught behind a lock.  That would match your 'per-statement' lock
> timeout concept, iiuc, though I think it might be more simply
> implemented.
>
>>> - Not a big fan of this:
>>>
>>> +    * See notes in PGSemaphoreLock.
>> Why? Copy&paste the *long* comment (a different one in each *_sema.c)
>> or omitting the comments is better? Suggest a better comment, and
>> it will be included.
> How about, for starters, there's more than one PGSemaphoreLock?  Second,
> as you'll note in posix_sema.c, it's useful to say more than just "look
> over there".  I'm not suggesting a mass copy/paste, but more than 4
> words would be valuable.

OK.

>
>>> - Do we really need TimestampTzPlusMicroseconds..?
>> Well, my code is the only user for this macro but it's cleaner
>> than explicitly doing
> To be honest, I was really wondering why TimestampTzPlusMilliseconds
> isn't sufficient, not suggesting that we litter the code with #ifdef's.

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

>
>     Thanks,
>
>         Stephen


--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de     http://www.postgresql.at/