Re: Is it time for triage on the open patches?

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Well, if you get to the point where you're done churning the code in
> the next week or so, I'm willing to do one or two more rounds of
> serious review, but if that doesn't get us there then I think we need
> to give up. The energy you've put into this is commendable, but we're
> about to start the third month of this CommitFest, and until we get
> this release at least to beta or so, we can't start any new
> CommitFests or branch the tree. That basically means that nothing
> else of mine is going to get committed until the current crop of
> patches are dealt with - or for a good while after, for that matter,
> but getting the current crop of patches dealt with is the first step.
> Of course, I also want to have a good release and I understand the
> necessity of spending time on other people's patches as well as my
> own, as I believe I've demonstrated, but I don't want to stay in that
> mode indefinitely, which I think is an understandable position.

This is a fair position, but I think it's a bit unfair to be applying
such pressure to just the command-triggers patch and not all the other
open issues. Hence, $SUBJECT: is it time to start forcing this
commitfest to a conclusion, and if so what kind of schedule are we
trying to set?

Personally, the open patches that I'm excited about getting into the
tree (or at least trying hard to) are:
* NULLs support in SP-GiST
* Caching constant stable expressions per execution
and not that much else. (I'm also interested in the pgsql_fdw patch
but I'm afraid that getting it to committable shape in the next week
or two may be unrealistic.) Probably other people have their own,
different shortlists.

(This is not to say that I'm objecting to any other patches, only that
I'd push to delay closing the CF to finish these, but not others.)

I think a reasonable way to proceed might be to get some consensus on
a short list of patches we're willing to try to push to completion,
then set a schedule accordingly, and then anything that doesn't get
done by the deadline gets kicked to 9.3.

Or we can just keep drifting. But the number of open patches that
are *not* Ready for Committer, nigh two months after the CF started,
is depressingly large.

regards, tom lane

On Fri, Mar 9, 2012 at 2:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> This is a fair position, but I think it's a bit unfair to be applying
> such pressure to just the command-triggers patch and not all the other
> open issues.  Hence, $SUBJECT: is it time to start forcing this
> commitfest to a conclusion, and if so what kind of schedule are we
> trying to set?

Just to be clear, it wasn't my intention to hold command triggers
specifically to a different standard - but I do differentiate between
small patches and big patches. Small patches that someone can get
committed with an hour's worth of review can be treated a little more
leniently than large patches that may take many cycles of review
adding up to days of effort, and I believe command triggers to be one
such patch.

> Personally, the open patches that I'm excited about getting into the
> tree (or at least trying hard to) are:
>        * NULLs support in SP-GiST
>        * Caching constant stable expressions per execution
> and not that much else.  (I'm also interested in the pgsql_fdw patch
> but I'm afraid that getting it to committable shape in the next week
> or two may be unrealistic.)  Probably other people have their own,
> different shortlists.

I'd like to get the two sepgsql patches done if possible. I'm going
to commit the rest of the DROP patch shortly, and the GUC for client
label I will review and commit if it seems like it's in good shape. I
would *like* to see Heikki's XLogInsert scaling patch committed, but
it seems like it's still too buggy for that, and I'm not sure how long
we should wait for it to get fixed; I also don't have plans to work on
it personally. It's hard to pick favorites among the rest; there are
a number of things I'd like to work on, but if it's just me working on
them it's going to take longer than I want to wait for them to get
done. There's been very little patch review going on, with a couple
of notable exceptions like Thom and Noah, and not a lot of new patch
versions from patch authors either, again with a few exceptions, like
Dimitri. So it's not terribly surprising that progress is very slow.
I'm not sure what to do about that, either: it doesn't seem very fair
to start booting patches things that are in relatively good shape, but
on the other hand I'm not willing to single-handedly (or even with
both hands) take on the task of reviewing everything that nobody else
is paying attention to.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> There's been very little patch review going on, with a couple
> of notable exceptions like Thom and Noah, and not a lot of new patch
> versions from patch authors either, again with a few exceptions, like
> Dimitri. So it's not terribly surprising that progress is very slow.
> I'm not sure what to do about that, either: it doesn't seem very fair
> to start booting patches things that are in relatively good shape, but
> on the other hand I'm not willing to single-handedly (or even with
> both hands) take on the task of reviewing everything that nobody else
> is paying attention to.

Right. IMO the point of setting a deadline would be to try to light a
fire under people who have been letting reviewing and patch-updating
slide. I don't want to arbitrarily boot a lot of small patches that
could have gotten done, but I don't want the active committers to be the
only ones pushing things to completion, either.

regards, tom lane

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Just to be clear, it wasn't my intention to hold command triggers
> specifically to a different standard - but I do differentiate between
> small patches and big patches. Small patches that someone can get
> committed with an hour's worth of review can be treated a little more
> leniently than large patches that may take many cycles of review
> adding up to days of effort, and I believe command triggers to be one
> such patch.

I share your view here, and in fact the code for the patch has been
updated in only two ways since 1/15: adding support for new commands and
reacting to review (refactoring, cleaning, features removal, fix the
glitch). That's the reason why I can see we're very near the end of it,
the code churn is about to be over now.

> There's been very little patch review going on, with a couple

Yeah, I'd like to get back reviewing soon too, obviously I've been
somehow more busy than expected.

> I'm not sure what to do about that, either: it doesn't seem very fair
> to start booting patches things that are in relatively good shape, but
> on the other hand I'm not willing to single-handedly (or even with
> both hands) take on the task of reviewing everything that nobody else
> is paying attention to.

It seems like February has seen lots of participants distracted away
from the commit fest, we should probably take this into account.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Fri, Mar 9, 2012 at 3:40 PM, Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr> wrote:
> I share your view here, and in fact the code for the patch has been
> updated in only two ways since 1/15: adding support for new commands and
> reacting to review (refactoring, cleaning, features removal, fix the
> glitch). That's the reason why I can see we're very near the end of it,
> the code churn is about to be over now.

Eh, maybe. I suspect you may be underestimating the amount of work
left to do, but I'll reserve judgement until I read a new version of
the patch.

> It seems like February has seen lots of participants distracted away
> from the commit fest, we should probably take this into account.

Sure, I got knocked out for a week by my trip to Japan, and Tom was
busy with other things, too, and Heikki went on vacation for a week.
But, really, I don't think that's the main problem. If people are
tired of working on the CommitFest, they're not going to get
reinvigorated just because we let it go on for another month.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Friday, March 09, 2012 10:13:15 PM Robert Haas wrote:
> If people are
> tired of working on the CommitFest, they're not going to get
> reinvigorated just because we let it go on for another month.
On that line: From Sundway onwards I do have time again to do reviewing. I am
not anybody is doing commitfest management right now, so I am asking here. Any
patch that can benefit from the level of review I can do?
If nobody raises a voice I will take a look at the next version of
commandtriggers first.

Andres

On Fri, Mar 9, 2012 at 7:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> I think a reasonable way to proceed might be to get some consensus on
> a short list of patches we're willing to try to push to completion,
> then set a schedule accordingly, and then anything that doesn't get
> done by the deadline gets kicked to 9.3.
>
> Or we can just keep drifting.  But the number of open patches that
> are *not* Ready for Committer, nigh two months after the CF started,
> is depressingly large.

* FOR KEY SHARE locks looks in very good shape and so I'm spending
time on that with a view to committing it next week if all goes well

* pg_stat_statements looks good also, I hope someone is looking at that

It's a good thing we have so many patches. We just need to organise
ourselves to ensure that we're working on priority items and spot
important things that are receiving no attention.

At this stage the CF app isn't helping us much. We need some way to
indicate who is actively working on review, not just a list of people
who have at some point reviewed it. Patches without committers will
likely suffer, so we need to be able to see the Committers column on
the display, so we know whether a patch needs one assigned.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Sat, Mar 10, 2012 at 9:04 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> * FOR KEY SHARE locks looks in very good shape and so I'm spending
> time on that with a view to committing it next week if all goes well

Álvaro is a committer and is perfectly capable of committing that
patch for himself, had we consensus on it. So far we don't, and I
don't think you can make a unilateral decision to the contrary. Maybe
that's not what you're proposing, but then what exactly are you
proposing?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> * pg_stat_statements looks good also, I hope someone is looking at that

I will take that one, if it ever gets marked RFC, but in the meantime
I plan to spend my time elsewhere.

> At this stage the CF app isn't helping us much. We need some way to
> indicate who is actively working on review, not just a list of people
> who have at some point reviewed it. Patches without committers will
> likely suffer, so we need to be able to see the Committers column on
> the display, so we know whether a patch needs one assigned.

I've kind of wished the Committer field could be seen in the list view
too, although I'm not sure what I'd give up for it (narrowing the other
fields doesn't sound good). Mainly what I wanted it for is to check
"is there anything I claimed and have not dealt with"; which is not
something I need to check every time I look, so perhaps some different
view could cover this need.

Another thing that would be really nice is the ability to sort open
items by last-activity time (disregarding the category labels), to make
it easier to spot which items are not receiving any attention.

regards, tom lane

On Sat, Mar 10, 2012 at 2:55 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Mar 10, 2012 at 9:04 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
>> * FOR KEY SHARE locks looks in very good shape and so I'm spending
>> time on that with a view to committing it next week if all goes well
>
> Álvaro is a committer and is perfectly capable of committing that
> patch for himself, had we consensus on it.  So far we don't, and I
> don't think you can make a unilateral decision to the contrary.  Maybe
> that's not what you're proposing, but then what exactly are you
> proposing?

I don't think I can express myself more clearly than the above quote.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Sat, Mar 10, 2012 at 3:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> * pg_stat_statements looks good also, I hope someone is looking at that
>
> I will take that one, if it ever gets marked RFC, but in the meantime
> I plan to spend my time elsewhere.
>
>> At this stage the CF app isn't helping us much. We need some way to
>> indicate who is actively working on review, not just a list of people
>> who have at some point reviewed it. Patches without committers will
>> likely suffer, so we need to be able to see the Committers column on
>> the display, so we know whether a patch needs one assigned.
>
> I've kind of wished the Committer field could be seen in the list view
> too, although I'm not sure what I'd give up for it (narrowing the other
> fields doesn't sound good).  Mainly what I wanted it for is to check
> "is there anything I claimed and have not dealt with"; which is not
> something I need to check every time I look, so perhaps some different
> view could cover this need.

Yes, that's a good idea.

> Another thing that would be really nice is the ability to sort open
> items by last-activity time (disregarding the category labels), to make
> it easier to spot which items are not receiving any attention.

And another.

My worry is things I care about, but may not be receiving the
attention they deserve (IMHO) so I can do something about them before
someone calls time. I guess that means I'd like to assign Importance
values to them as well for my own use.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 10 March 2012 15:47, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> * pg_stat_statements looks good also, I hope someone is looking at that
>
> I will take that one, if it ever gets marked RFC, but in the meantime
> I plan to spend my time elsewhere.

It has been marked RFC now.

--
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services