Thread: Is it time for triage on the open patches?
Robert Haas <robertmhaas@gmail.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@sss.pgh.pa.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@gmail.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@gmail.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@2ndquadrant.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@sss.pgh.pa.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@2ndquadrant.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@2ndQuadrant.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@gmail.com> wrote: > On Sat, Mar 10, 2012 at 9:04 AM, Simon Riggs <simon@2ndquadrant.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@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.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@sss.pgh.pa.us> wrote: > Simon Riggs <simon@2ndQuadrant.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