Re: Lock Wait Statistics (next commitfest) - Mailing list pgsql-hackers

From Mark Kirkwood
Subject Re: Lock Wait Statistics (next commitfest)
Date
Msg-id 4B8988DF.1060703@catalyst.net.nz
Whole thread Raw
In response to Re: Lock Wait Statistics (next commitfest)  (Greg Smith <greg@2ndquadrant.com>)
Responses Re: Lock Wait Statistics (next commitfest)
Re: Lock Wait Statistics (next commitfest)
List pgsql-hackers
Greg Smith wrote:
> Mark Kirkwood wrote:
>> Greg Smith wrote:
>>> Returned with feedback in October after receiving a lot of review, 
>>> no updated version submitted since then:
>>>
>>> https://commitfest.postgresql.org/action/patch_view?id=98
>>>
>>
>> Hmm - I would say a bit of review rather than a lot :-)
>
> It looks like you got useful feedback from at least three people, and 
> people were regularly looking at your patch in some form for about 
> three months.  That's a lot of review.  In many other open-source 
> projects, your first patch would have been rejected after a quick look 
> as unsuitable and that would have been the end of things for you.  I 
> feel lucky every time I get a volunteer to spend time reading my work 
> and suggesting how it could be better; your message here doesn't seem 
> to share that perspective.

I don't mean to be ungrateful about the actual reviews at all - and I 
did value the feedback received (which I hope was reasonably clear in 
the various replies I sent). I sense a bit of attacking the messenger in 
your tone... I've submitted several patches to Postgres in the past, and 
have previously always enjoyed the experience, and I do get the culture 
- being a volunteer myself.
>
> To lower your frustration level next time, make sure to label the 
> e-mail and the entry on the CommitFest app with the magic abbreviation 
> "WIP" and this shouldn't be so much of an issue.  The assumption for 
> patches is that someone submitted them as commit candidates, and 
> therefore they should be reviewed to that standard, unless clearly 
> labeled otherwise.  You briefly disclaimed yours as not being in that 
> category in the initial text of your first message, but it was easy to 
> miss that, particularly once it had been >8 months from when that 
> messages showed up and it was still being discussed.
>

LOL - I said a bit - it was only a little, connected with the commit vs 
review confusion. I think I just got caught in the bedding in time for 
the new development processes, I was advised to add it to the 
commitfests, and them advised that it should not have been at a later 
time. Yes, a bit frustrating at the time but not earth shattering at 
all. I'm mentioning it now mainly to help clarify the situation for 
anyone else wanting a WIP patch reviewed! In my case while labeling as 
WIP could well have helped - the (pretty short) message accompanying the 
patch is very clear that there is stuff to do - using the magic phrase 
"...merely to spark discussion..." - as were all the followup 
accompanying ones, with phrases like "still todo...". So yes, next time 
I'll label any patches more clearly, reviewers need to read the text of 
the patch they are about to review (note that most did).

> If you wanted to pick this back up again, I'd think that a look at 
> what's been happening with the lock_timeout GUC patch would be 
> informative--I'd think that has some overlap with the sort of thing 
> you were trying to do.
>
> FYI, I thought your patch was useful, but didn't spent time on it 
> because it's not ambitious enough.  I would like to see statistics on 
> a lot more types of waiting than just locks, and keep trying to find 
> time to think about that big problem rather than worrying about the 
> individual pieces of it.
>
Excellent thanks - that is exactly the sort of comment that would have 
been very valuable to have made at the time (echo's Gokul's recent one 
interestingly enough). After all if enough people share this view, then 
clearly I need to either forget about the current patch, or design 
something more ambitious!

regards

Mark



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: NaN/Inf fix for ECPG
Next
From: Michael Meskes
Date:
Subject: Re: ECPG, two varchars with same name on same line