Thread: CommitFest 2011-01 as of 2011-02-04
Here's where I think we are with this CommitFest. We have committed 45 patches and returned with feedback or rejected 23. There are 30 remaining patches, every single one of which has been reviewed. 20 of those are marked Ready for Committer; 5 are marked Waiting on Author; 5 are marked Needs Review. However, again, even the ones that are marked as Needs Review have in fact been reviewed multiple times, in many cases by multiple people. Thanks to all who have contributed to the reviewing effort, especially Stephen Frost, Hitoshi Hirada, Alex Hunsaker, Noah Misch, and Itagaki Takahiro. I respectfully submit that every patch which is not yet Ready for Committer is in that state not because of any neglect on the part of anyone in the community, but because it's got problems. It isn't done; it turned out to be more complicated than anticipated; it wasn't updated in a timely fashion; and/or we had difficulty reaching agreement on the best way forward (and still haven't). Most of these patches should probably be deferred to 9.2. So there are two basic difficulties with wrapping the CommitFest up. One is that the 20 patches which are marked Ready for Committer really deserve to be looked at by a committer, and hopefully committed. Unfortunately, my ability to help in this area will be somewhat limited, because at least half of the remaining patches are in areas that I know nothing about (e.g. 7 PL/python patches). The second problem is that the patches that are in serious trouble including synchronous replication and SQL/MED, which are key features I know we're all hoping to have for 9.2. However, we have to face the fact that getting these features in may involve quite a bit of delay. With respect to SQL/MED, Heikki tells me that he is working on the core patch, and I am hopeful that will be committed soon. However, file_fdw is in pretty serious trouble because (1) the copy API patch that it depends on still isn't committed and (2) it's going to be utterly broken if we don't do something about the client_encoding vs. file_encoding problem; there was a patch to do that in this CF, but we gave up on it. And postgresql_fdw was hacked up by Heikki but he didn't seem to have much confidence in it and no one else has reviewed his hacked-up version. With respect to synchronous replication, Dan Farina and I have extracted some of the less-intrusive bits of that patch. One of those changes (standby replies) is committed, though it seems to need a bit of fixup, and the other two are awaiting review. Simon also reported that he is working on this, but I'm not certain of the specifics. Based on the looking at that patch that I did so far, it certainly needs cleanup, and there may be some more serious architectural issues that need to be addressed also. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote:
> Here's where I think we are with this CommitFest.
Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04
I'm gonna go out on a limb and hope you meant '2011-02-14' there. :)
> So there are two basic difficulties with wrapping the CommitFest up.
I have to say that I've always been a bit suprised by the idea that the
CommitFest is intended to be done and all patches *committed* at the end
of the month. It's been working really rather well, which is due in
great part to the excellent CF managers (thanks again for being that,
again). That said, we have quite a few non-committer reviewers who
provide good feedback and move the patch back to 'waiting for author'
and that whole process takes a while.
Perhaps a thought for next time would be to offset things a bit. eg:
CF 2011-03 (or whatever):
2011-02-14: Patches should all be submitted
2011-02-14: Reviewers start
2011-03-01: Committers start w/ 'Ready for Committer' patches
2011-03-14: Patches not marked 'Ready for Committer' get bounced
2011-03-31: All patches committed
I'm not against the 'waiting on author' approach, but I do feel like if
we're going to continue to have it, we need to spread it out a bit more.
I do think this would place more work on the CF manager, unfortunately,
but I'd hope that they would primairly be focused on managing the
reviews and not be as busy during the last 2 weeks. Maybe one day I'll
be brave enough to offer to manage one and see. :)
Thanks again, Robert, you've done an excellent job managing the CF.
Stephen
Robert Haas <robertmhaas@gmail.com> writes: > We have committed 45 patches and returned with feedback or rejected > 23. There are 30 remaining patches, every single one of which has > been reviewed. 20 of those are marked Ready for Committer; 5 are > marked Waiting on Author; 5 are marked Needs Review. However, again, I just took the liberty to mark the 2 extension patches as commited, even if we have some more "details" to fix. Well if we include the PL stuff, it's detail and a half, but I though marking them commited would help us following here. I hope it does. Will sleep on that. Now. :) I would like to see this commit fest extended by at least a week, maybe two, like has been proposed before. The overall rhythm has been quite impressive, and I'm not seeing such a slowdown that it's useless to try continuing. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost <sfrost@snowman.net> wrote: > I have to say that I've always been a bit suprised by the idea that the > CommitFest is intended to be done and all patches *committed* at the end > of the month. It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). That said, we have quite a few non-committer reviewers who > provide good feedback and move the patch back to 'waiting for author' > and that whole process takes a while. > > Perhaps a thought for next time would be to offset things a bit. eg: > > CF 2011-03 (or whatever): > 2011-02-14: Patches should all be submitted > 2011-02-14: Reviewers start > 2011-03-01: Committers start w/ 'Ready for Committer' patches > 2011-03-14: Patches not marked 'Ready for Committer' get bounced > 2011-03-31: All patches committed > > I'm not against the 'waiting on author' approach, but I do feel like if > we're going to continue to have it, we need to spread it out a bit more. > I do think this would place more work on the CF manager, unfortunately, > but I'd hope that they would primairly be focused on managing the > reviews and not be as busy during the last 2 weeks. Maybe one day I'll > be brave enough to offer to manage one and see. :) > > Thanks again, Robert, you've done an excellent job managing the CF. > > Stephen > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1.4.10 (GNU/Linux) > > iEYEARECAAYFAk1ZXTQACgkQrzgMPqB3kiiLAQCfUVusKmhcQW1KNGQwZSFpdONx > G4oAnjzPLSQpyaounlTMrumdoQe58yA/ > =zuCX > -----END PGP SIGNATURE----- > > -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Sorry for the previous, content-free reply. On Mon, Feb 14, 2011 at 11:49 AM, Stephen Frost <sfrost@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> Here's where I think we are with this CommitFest. > > Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04 > > I'm gonna go out on a limb and hope you meant '2011-02-14' there. :) Yeah, sorry. >> So there are two basic difficulties with wrapping the CommitFest up. > > I have to say that I've always been a bit suprised by the idea that the > CommitFest is intended to be done and all patches *committed* at the end > of the month. It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). That said, we have quite a few non-committer reviewers who > provide good feedback and move the patch back to 'waiting for author' > and that whole process takes a while. It does, but frankly I don't see much reason to change it, since it's been working pretty well on the whole. Andrew was on point when he mentioned that it's not obvious what committers get out of working on other people's patches. Obviously, the answer is, well, they get a better PostgreSQL, and that's ultimately good for all of us. But the trickiest part of this whole process is that, on the one hand, it's not fair for committers to ignore other people's patches, but on the other hand, it's not fair to expect committers to sacrifice getting their own projects done to get other people's projects done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote:
> But the
> trickiest part of this whole process is that, on the one hand, it's
> not fair for committers to ignore other people's patches, but on the
> other hand, it's not fair to expect committers to sacrifice getting
> their own projects done to get other people's projects done.
It wasn't my intent to ask the committers to do more but rather to
change the timings a bit so that when committers begin their "commitfest
month", the patches are already in better quality and more apt to be
ready for committer.
Anyhow, was just a thought.
Thanks,
Stephen
On mån, 2011-02-14 at 11:49 -0500, Stephen Frost wrote: > Perhaps a thought for next time would be to offset things a bit. eg: > > CF 2011-03 (or whatever): > 2011-02-14: Patches should all be submitted > 2011-02-14: Reviewers start > 2011-03-01: Committers start w/ 'Ready for Committer' patches > 2011-03-14: Patches not marked 'Ready for Committer' get bounced > 2011-03-31: All patches committed > > I'm not against the 'waiting on author' approach, but I do feel like > if we're going to continue to have it, we need to spread it out a bit > more. I don't think it is realistic to add even more dates and bounds and guidelines. People are already widely ignoring the current ones. If you want to have the ability the bounce things more aggressively, I'd argue for shorter and more frequent commitfests. Say, one week per month.
On Tue, Feb 15, 2011 at 01:27, Robert Haas <robertmhaas@gmail.com> wrote: > However, file_fdw is in pretty serious trouble because (1) the copy > API patch that it depends on still isn't committed and (2) it's going > to be utterly broken if we don't do something about the > client_encoding vs. file_encoding problem; there was a patch to do > that in this CF, but we gave up on it. Will we include the copy API patch in 9.1 even if we won't have file_fdw? Personally, I want to include the APIs because someone can develop file_fdw as a third party extension for 9.1 using the infrastructure. The extension will lack of file encoding support, but still useful for many cases. -- Itagaki Takahiro
On Tue, Feb 15, 2011 at 3:31 AM, Itagaki Takahiro <itagaki.takahiro@gmail.com> wrote: > On Tue, Feb 15, 2011 at 01:27, Robert Haas <robertmhaas@gmail.com> wrote: >> However, file_fdw is in pretty serious trouble because (1) the copy >> API patch that it depends on still isn't committed and (2) it's going >> to be utterly broken if we don't do something about the >> client_encoding vs. file_encoding problem; there was a patch to do >> that in this CF, but we gave up on it. > > Will we include the copy API patch in 9.1 even if we won't have file_fdw? > Personally, I want to include the APIs because someone can develop file_fdw > as a third party extension for 9.1 using the infrastructure. The extension > will lack of file encoding support, but still useful for many cases. I've been kind of wondering why you haven't already committed it. If you're confident that the code is in good shape, I don't particularly see any benefit to holding off. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 02/15/2011 06:55 AM, Robert Haas wrote: > On Tue, Feb 15, 2011 at 3:31 AM, Itagaki Takahiro > <itagaki.takahiro@gmail.com> wrote: >> On Tue, Feb 15, 2011 at 01:27, Robert Haas<robertmhaas@gmail.com> wrote: >>> However, file_fdw is in pretty serious trouble because (1) the copy >>> API patch that it depends on still isn't committed and (2) it's going >>> to be utterly broken if we don't do something about the >>> client_encoding vs. file_encoding problem; there was a patch to do >>> that in this CF, but we gave up on it. >> Will we include the copy API patch in 9.1 even if we won't have file_fdw? >> Personally, I want to include the APIs because someone can develop file_fdw >> as a third party extension for 9.1 using the infrastructure. The extension >> will lack of file encoding support, but still useful for many cases. > I've been kind of wondering why you haven't already committed it. If > you're confident that the code is in good shape, I don't particularly > see any benefit to holding off. > +10. The sooner the better. cheers andrew
robertmhaas@gmail.com (Robert Haas) writes: > It does, but frankly I don't see much reason to change it, since it's > been working pretty well on the whole. Andrew was on point when he > mentioned that it's not obvious what committers get out of working on > other people's patches. Obviously, the answer is, well, they get a > better PostgreSQL, and that's ultimately good for all of us. But the > trickiest part of this whole process is that, on the one hand, it's > not fair for committers to ignore other people's patches, but on the > other hand, it's not fair to expect committers to sacrifice getting > their own projects done to get other people's projects done. I had two interesting germane comments in my RSS feed this morning, both entitled "Please send a patch" http://www.lucas-nussbaum.net/blog/?p=630 Where Lucas suggests that, when someone requests an enhancement, the retort "Please send a patch" mayn't be the bestidea, because the one receiving the requests may be many times better at contributing such changes than the one makingthe request. http://hezmatt.org/~mpalmer/blog/general/please_send_a_patch.html "On the other hand, Lucas, remember that each time you ask someone to take some time to implement your pet feature request,you take some time away from her that could be used to contribute something in an area where she gives a damn." These are *both* true statements, and, in order to grow the community that is capable of enhancing the system, there is merit to the careful application of both positions. There's stuff that Tom should do :-). And absent the general availability of cloning machines, we need to have people improving their skills so that there are more that are capable of submitting (and evaluating and committing) usable patches. -- wm(X,Y):-write(X),write('@'),write(Y). wm('cbbrowne','gmail.com'). http://linuxdatabases.info/info/languages.html Signs of a Klingon Programmer - 14. "Our competitors are without honor!"
On Wed, Feb 16, 2011 at 02:14, Andrew Dunstan <andrew@dunslane.net> wrote: >> I've been kind of wondering why you haven't already committed it. If >> you're confident that the code is in good shape, I don't particularly >> see any benefit to holding off. > > +10. The sooner the better. Thanks comments. I've applied the COPY API patch. -- Itagaki Takahiro
On Mon, Feb 14, 2011 at 09:49, Stephen Frost <sfrost@snowman.net> wrote: > * Robert Haas (robertmhaas@gmail.com) wrote: >> Here's where I think we are with this CommitFest. > > Subject: Re: [HACKERS] CommitFest 2011-01 as of 2011-02-04 > > I'm gonna go out on a limb and hope you meant '2011-02-14' there. :) > >> So there are two basic difficulties with wrapping the CommitFest up. > > [ ... ] It's been working really rather well, which is due in > great part to the excellent CF managers (thanks again for being that, > again). [ ... ] > Thanks again, Robert, you've done an excellent job managing the CF. I'd like to second this sentiment. I'm positive no one can really appreciate the amount of work that goes into managing a commitfest other than the elite few who have. I imagine its not unlike being a mother: constantly bitching, ( out of necessity mind you-- don't commit that! its HOT!, did you finish all of your reviews? no? there are starving people in china! etc.) and yet underrated, under-thanked and often undervalued. Anyway, good job and thanks for volunteering to be the "bad" guy that tries to keep the 9.1 time table sane.
On Tue, 2011-02-15 at 06:49 +0200, Peter Eisentraut wrote: > On mån, 2011-02-14 at 11:49 -0500, Stephen Frost wrote: > > Perhaps a thought for next time would be to offset things a bit. eg: > > > > CF 2011-03 (or whatever): > > 2011-02-14: Patches should all be submitted > > 2011-02-14: Reviewers start > > 2011-03-01: Committers start w/ 'Ready for Committer' patches > > 2011-03-14: Patches not marked 'Ready for Committer' get bounced > > 2011-03-31: All patches committed > > > > I'm not against the 'waiting on author' approach, but I do feel like > > if we're going to continue to have it, we need to spread it out a bit > > more. > > I don't think it is realistic to add even more dates and bounds and > guidelines. Though I agree with that, I see Stephen's suggested sequence of events as a useful one for CFs managers. > People are already widely ignoring the current ones. I don't think people are ignoring things deliberately. List traffic is high and unless you are 100% full time on this, you can't possibly hope to read them all, respond to the relevant ones and do your own work too. Especially when many folk have a day job as well. > If you want to have the ability the bounce things more aggressively, I'd > argue for shorter and more frequent commitfests. Say, one week per > month. That is the blink of an eye for me, so don't want more frequent CFs. The focus should be on organising ourselves so that the most number of high quality features get into Postgres. I don't see anything to be gained by bouncing things aggressively; strict time-boxing works on 2 weekly cycles, not on annual ones. We're doing OK. -- Simon Riggs http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services
On Wed, 16 Feb 2011 11:22:04 +0900 Itagaki Takahiro <itagaki.takahiro@gmail.com> wrote: > Thanks comments. I've applied the COPY API patch. When I've used COPY TO for testing file_fdw, I got wrong result. # Actually csv_branches has only 10 rows. postgres=# copy (select * from csv_branches) to '/home/hanada/DB/BINARY/branches.bin' with binary; COPY 9187201950435737481 It would be because DR_copy's processed is not initialized in CreateCopyDestReceiver(). Please see attached patch. Regards, -- Shigeru Hanada
Attachment
On Fri, Feb 18, 2011 at 13:15, Shigeru HANADA <hanada@metrosystems.co.jp> wrote: > When I've used COPY TO for testing file_fdw, I got wrong result. > It would be because DR_copy's processed is not initialized in > CreateCopyDestReceiver(). Please see attached patch. Oops, thanks, applied. -- Itagaki Takahiro