Обсуждение: 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

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