Обсуждение: 2016-01 Commitfest

Hello,

As discussed, I will be managing the commitfest process this time
around.  Please, everyone, make sure patches are registered in the app
today.  As of this instant, we have:
Needs review: 73.Waiting on Author: 5.Ready for Committer: 6.Committed: 5.
Total: 89. 

Thanks,

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Here are the current numbers, now that the commitfest has actually
closed for business:
Needs review: 79.Waiting on Author: 5.Ready for Committer: 6.Committed: 8.
Total: 98.

Of those RfC patches, one ("Default Roles") doesn't actually seem ready
to commit, since there's been some significant discussion regarding the
underlying design.  I'm setting it back to Needs Review while the
discussion continues.

Patch "Speedup timestamp/time/date output functions" in RfC status has a
committer assigned; I hope this one to be over soon.

The bulk, of course, is still in need of input from reviewers, so let's
keep it moving!

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

These are the numbers after one week of commitfest work:
Needs review: 65.Waiting on Author: 14.Ready for Committer: 6.Committed: 14.
Total: 99. 

The attentive reader might notice that we grew one more patch since last
week, which is the "VACUUM progress checker" thingy that has been under
active review.

We went from 8 committed patches to 14.  At the current rate of 6
patches per week it would take us 12 weeks to close the commitfest,
which doesn't sound very good.

There are a number of patches in Needs-review state which haven't seen
any pgsql-hackers activity in a long while.  I'm particularly concerned
about the following patches:

* Support multiple synchronous standby servers
* Access method extendability
* Partial sort
* add 'waiting for replication' to pg_stat_activity.state
* More stable query plans via more predictable column statistics
* Statistics for array types
* Declarative partitioning
* Table Partition + Join Pushdown
* multivariate statistics

It would be very helpful of a reviewer to look at those patches.


Some committers have assigned patches to themselves:

* Andres Freund Speedup timestamp/time/date output functions
* Peter Eisentraut remove wal_level archive
* Tom Lane Rework index access method interface
* Teodor Sigaev New gist vacuum
* Stephen Frost Default Roles
* Simon Riggs Fix handling on XLOG_RUNNING_XACTS generated by bgwriter on idle systems

I assume this means they intend to commit them in some reasonable
timeframe (possibly after some rework).  If this is not the case, please
let us know.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Jan 11, 2016 at 6:38 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> * Partial sort

That shouldn't have been in "needs review" state. Fixed.


-- 
Peter Geoghegan

Two weeks into the commitfest, things have moved a bit:
Needs review: 53.Waiting on Author: 20.Ready for Committer: 10.Committed: 16.
Total: 99.        https://commitfest.postgresql.org/8/

We have two committed patches since last report -- not a lot for a whole
week.  We've managed to review a few patches though: WoA went from 14 to
20.  Patch authors have something to do rather just twiddle their
thumbs^W^W^W review others' patches.

Remember: reviewing patches where somebody else has marked themselves as
reviewer is still welcome.  You could spot things the other person might
miss.

Committers: There are patches for you to push!  Go pick one up right now!
(And remember: marking yourself as committer for a patch makes it
unlikely for anybody else to review/commit it.  If you do not intend to
review+commit such a patch soon, it's probably better to remove yourself
as committer.)

Many patches seem stalled without review:  Table partition + join pushdown    https://commitfest.postgresql.org/8/325/
multivariatestatistics        https://commitfest.postgresql.org/8/450/  Implement failover on libpq level
https://commitfest.postgresql.org/8/389/ Unique Joins                https://commitfest.postgresql.org/8/129/  Replace
buffermanager spinlock with atomic operations                      https://commitfest.postgresql.org/8/408/
 

Reviews of those patches would be useful, to keep things moving forward.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Into its third week, this commitfest is looking like this:
Needs review: 41.Waiting on Author: 24.Ready for Committer: 10.Committed: 23.Rejected: 1.
Total: 99. 

The number of committed patches continues to grow slowly but steadily,
which is a good sign -- and in the past week it grew even faster than
the week before, which is a nice indicator.  Also, the number of patches
waiting on author keeps growing, which is a good indicator for "us"
(reviewers) because it means authors have actionable information; it
does mean patch authors have been slacking, though, so please do pick up
your stuff and get it up to snuff.  Hopefully we will be receiving
updates patches soon.

We still have 41 patches that haven't gotten enough review though.  The
bad part about it is that there's a number of patches that have been
bouncing for many commitfests now.  Here's a list of the patches with
the most such actions (both in Needs Review and Ready for Committer
state):

Five "Moved to next commitfest"
* https://commitfest.postgresql.org/8/49/ Default Roles

Four "Moved to next commitfest"
* https://commitfest.postgresql.org/8/129/ Unique Joins

Three "Moved to next commitfest"
* https://commitfest.postgresql.org/8/169/ postgres_fdw: Options to set fetch_size at the server and table level.

* https://commitfest.postgresql.org/8/180/ Atomic commit support for foreign data wrappers

* https://commitfest.postgresql.org/8/189/ extends pgbench expressions with functions

* https://commitfest.postgresql.org/8/210/ improving join estimates using FK

* https://commitfest.postgresql.org/8/260/ checkpoint continuous flushing

I list these now because it is fair to get them processed before others
that are more recent.  (If we do not want them at all, let's reject
them.)  This list does not include patches that were closed as Returned
with Feedback and resubmitted later as a new patch entry; I don't think
the current CF process lets me know that.

Please keep in mind that any patch that's been Waiting on Author for too
long(*) may be closed as Returned with Feedback rather than moved to the
next commitfest, so make sure you send updated patches.  Patches marked
Needs Review but that have actually had reviews on list may suffer the
same fate, depending on how extensive the rework needed is.

Due to FOSDEM, I'm unlikely to be doing the closing action exactly on
January 31st, but it will be near that date.

(*) "Too long" is arbitrarily defined by your beloved commitfest
manager.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2016-01-25 13:36:04 -0300, Alvaro Herrera wrote:
> We still have 41 patches that haven't gotten enough review though.  The
> bad part about it is that there's a number of patches that have been
> bouncing for many commitfests now.  Here's a list of the patches with
> the most such actions (both in Needs Review and Ready for Committer
> state):

> * https://commitfest.postgresql.org/8/260/
>   checkpoint continuous flushing

FWIW, I've been working and benchmarking this a lot over the last
weeks.  After finally nailing down the performance regression due to wal
writer, thins are looking good.  I plan to post an updated version soon.

Andres

Andres Freund wrote:
> On 2016-01-25 13:36:04 -0300, Alvaro Herrera wrote:
> > We still have 41 patches that haven't gotten enough review though.  The
> > bad part about it is that there's a number of patches that have been
> > bouncing for many commitfests now.  Here's a list of the patches with
> > the most such actions (both in Needs Review and Ready for Committer
> > state):
> 
> > * https://commitfest.postgresql.org/8/260/
> >   checkpoint continuous flushing
> 
> FWIW, I've been working and benchmarking this a lot over the last
> weeks.  After finally nailing down the performance regression due to wal
> writer, thins are looking good.  I plan to post an updated version soon.

Excellent.  I assume the walwriter bug is going to get fixed separately,
probably backpatched too?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On 2016-01-25 15:02:02 -0300, Alvaro Herrera wrote:
> Andres Freund wrote:
> > On 2016-01-25 13:36:04 -0300, Alvaro Herrera wrote:
> > > * https://commitfest.postgresql.org/8/260/
> > >   checkpoint continuous flushing
> > 
> > FWIW, I've been working and benchmarking this a lot over the last
> > weeks.  After finally nailing down the performance regression due to wal
> > writer, thins are looking good.  I plan to post an updated version soon.
> 
> Excellent.  I assume the walwriter bug is going to get fixed separately,
> probably backpatched too?

I definitely have it as a separate patch, and plan to commit it as
such. But I'm doubtful about backpatching it - without the controlled
sorting & flushing the impact isn't *that* big (i.e. not a factor of
5-10), and it could also cause performance regressions. We could later
redecide, but I'd rather focus on working on master.

Andres

On Mon, Jan 25, 2016 at 11:36 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> We still have 41 patches that haven't gotten enough review though.  The
> bad part about it is that there's a number of patches that have been
> bouncing for many commitfests now.  Here's a list of the patches with
> the most such actions (both in Needs Review and Ready for Committer
> state):
>
> Five "Moved to next commitfest"
> * https://commitfest.postgresql.org/8/49/
>   Default Roles

This patch has gotten some feedback relatively recently, specifically
to question whether we want it.  I think it's fair to say that the
reaction to this proposal has never been cold nor more than lukewarm.

> Four "Moved to next commitfest"
> * https://commitfest.postgresql.org/8/129/
>   Unique Joins

I've been hoping Tom would pick this up.  If he doesn't, I will look
at it eventually, but it's not on my top 20 list of things to get
committed at this point.  I think it's worthwhile, but I just don't
have the bandwidth.

> Three "Moved to next commitfest"
> * https://commitfest.postgresql.org/8/169/
>   postgres_fdw: Options to set fetch_size at the server and table level.

Just reviewed that.

> * https://commitfest.postgresql.org/8/180/
>   Atomic commit support for foreign data wrappers

I think this needs considerably more work, but it probably needs a
series of detailed reviews first.  There's also some question about
how this fits into the distributed transaction manager work by the
Postgres Pro guys.

> * https://commitfest.postgresql.org/8/189/
>   extends pgbench expressions with functions

I looked at this in the past and felt that the design was not good.
It's been revised since then, but I haven't gotten back around to look
at it again.  I think this has gotten a fair amount of discussion but
I'm not sure we've converged on something really solid yet.

> * https://commitfest.postgresql.org/8/210/
>   improving join estimates using FK

This thread seems to end with some author-reviewer discussion that petered out.

> * https://commitfest.postgresql.org/8/260/
>   checkpoint continuous flushing

Andres FTW.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jan 25, 2016 at 11:36 AM, Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> Four "Moved to next commitfest"
>> * https://commitfest.postgresql.org/8/129/
>> Unique Joins

> I've been hoping Tom would pick this up.  If he doesn't, I will look
> at it eventually, but it's not on my top 20 list of things to get
> committed at this point.  I think it's worthwhile, but I just don't
> have the bandwidth.

I agree that this is on my turf, but like Robert I don't have time for
it right now.  Sooner or later I'll get to it.
        regards, tom lane

Hello Andres,

> FWIW, I've been working and benchmarking this a lot over the last weeks.

I'm running a lot of tests as weel, on HDDs. It is basically always better 
with the patch, although sometimes not sorting but flushing is better than 
both, which suggest that the gucs should be kept just in case.

> After finally nailing down the performance regression due to wal writer, 
> thins are looking good.  I plan to post an updated version soon.

Good. The last version sent (14?) does not apply cleanly. I'm looking 
forward to have another look at an updated version.

-- 
Fabien.

Hi,

Status summary:Needs review: 7.Ready for Committer: 11.Committed: 28.Moved to next CF: 23.Rejected: 2.Returned with
Feedback:28.
 
Total: 99. 

I just closed a large number of patches in the 2006-01 commitfest as
"returned with feedback".  The vast majority of those were in "waiting
on author"; I verified that the threads had posted something to the
author and the author had not yet replied.  Since we're past the closing
point for the commitfest, that's fair.  Patch authors are welcome to
submit a new version of their patch, which can be considered during the
next commitfest.  In this group I also included a number of "Needs
review" patches which had gotten some feedback and were waiting for a
new version.

Also, I some patches that have actually gotten some feedback but a new
version was posted afterwards I moved to the next commitfest.  We're not
waiting on the author to post a new version, but we've already given
some feedback so we're not obliged to keep them in the current
commitfest.

A number of patches still remain in the current commitfest.  11 of them
are marked as "ready for committer" so supposedly some committer should
grab them and push them.

7 are in "needs review" state; I just checked and most of those already
received some feedback and it seems fair to close them as RwF; new
versions can be submitted to next commitfest.  Two of them didn't,
AFAICS:
 https://commitfest.postgresql.org/8/372/    \crosstabview (previously: \rotate) in psql for crosstab-style display
https://commitfest.postgresql.org/8/323/   Statistics for array types
 

and for closure it would be good to either send them feedback if they
need rework or mark them as ready-for-committer.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> A number of patches still remain in the current commitfest.  11 of them
> are marked as "ready for committer" so supposedly some committer should
> grab them and push them.

The "SET ROLE hook" patch should be moved to RWF state; it's not going
to be committed in anything like its current form, per the last two
messages in that thread.

The reason the "remove wal_level archive" patch hasn't been committed
is that there is just about zero consensus on whether it's a good idea.
I don't see that getting resolved soon, and would not expect that it
will get committed in this CF.  Don't know what action is appropriate
on the CF entry, but RFC status seems misleading.

Heikki and/or Andres have their names on three of the remaining RFC
patches; it's unlikely any other committer will touch those patches
unless they take their names off.
        regards, tom lane

    Alvaro Herrera wrote:

>  https://commitfest.postgresql.org/8/372/
>     \crosstabview (previously: \rotate) in psql for crosstab-style display

About this one, the code is no longer moving, the latest addition was
regression tests a couple days ago.

I think it should be moved to the next CF, to be hopefully promoted to
Ready for Committer soon.

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

<p dir="ltr"><br /> Dne 3. 2. 2016 20:51 napsal uživatel "Daniel Verite" <<a
href="mailto:daniel@manitou-mail.org">daniel@manitou-mail.org</a>>:<br/> ><br /> >         Alvaro Herrera
wrote:<br/> ><br /> > >  <a
href="https://commitfest.postgresql.org/8/372/">https://commitfest.postgresql.org/8/372/</a><br/> > >   
 \crosstabview(previously: \rotate) in psql for crosstab-style display<br /> ><br /> > About this one, the code
isno longer moving, the latest addition was<br /> > regression tests a couple days ago.<br /> ><br /> > I
thinkit should be moved to the next CF, to be hopefully promoted to<br /> > Ready for Committer soon.<p dir="ltr">I
hopeit too.I am sorry, my way to Moscow was little bit longer than I expected. I have not any open questions or notes,
justwould to recheck final version.<p dir="ltr">Regards<p dir="ltr">Pavel<br /> ><br /> > Best regards,<br />
>--<br /> > Daniel Vérité<br /> > PostgreSQL-powered mailer: <a
href="http://www.manitou-mail.org">http://www.manitou-mail.org</a><br/> > Twitter: @DanielVerite<br /> ><br />
><br/> > --<br /> > Sent via pgsql-hackers mailing list (<a
href="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<br/> > To make changes to your
subscription:<br/> > <a
href="http://www.postgresql.org/mailpref/pgsql-hackers">http://www.postgresql.org/mailpref/pgsql-hackers</a><br/> 

On Wed, Feb 03, 2016 at 06:18:02PM +0100, Alvaro Herrera wrote:
> I just closed a large number of patches in the 2006-01 commitfest as
> "returned with feedback".  The vast majority of those were in "waiting
> on author"; I verified that the threads had posted something to the
> author and the author had not yet replied.  Since we're past the closing
> point for the commitfest, that's fair.  Patch authors are welcome to
> submit a new version of their patch, which can be considered during the
> next commitfest.  In this group I also included a number of "Needs
> review" patches which had gotten some feedback and were waiting for a
> new version.
> 
> Also, I some patches that have actually gotten some feedback but a new
> version was posted afterwards I moved to the next commitfest.  We're not
> waiting on the author to post a new version, but we've already given
> some feedback so we're not obliged to keep them in the current
> commitfest.

Thank you.

On Wed, Feb 3, 2016 at 10:00 PM, Noah Misch <noah@leadboat.com> wrote:
> Thank you.

+1.

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

On 2016/02/04 12:04, Robert Haas wrote:
> On Wed, Feb 3, 2016 at 10:00 PM, Noah Misch <noah@leadboat.com> wrote:
>> Thank you.

> +1.

Thank you!

Best regards,
Etsuro Fujita

On 2016-02-03 12:32:47 -0500, Tom Lane wrote:
> Heikki and/or Andres have their names on three of the remaining RFC
> patches; it's unlikely any other committer will touch those patches
> unless they take their names off.

If you want to take over the timestamp patch, please feel free to do
so. Otherwise I'll get to it in ~10 days.

I'm working on/off on the checkpointer flushing thing, so I don't think
it makes sense for somebody else to take that over at this point.

Andres

Hello Andres,

> I'm working on/off on the checkpointer flushing thing, so I don't think
> it makes sense for somebody else to take that over at this point.

Yep.

I still wish you could share your current working version? The last 
version sent is from November 11th, and I think someone said that it does 
not apply cleanly anymore on head.

-- 
Fabien.

Hi everybody,

I just closed the last few remaining items in the commitfest.  This is
the final summary:
Committed: 32.Moved to next CF: 32.Rejected: 2.Returned with Feedback: 33.
Total: 99. 

I think we did a fairly decent job this time around: we only passed a
third of the patches to the upcoming commitfest, which seems pretty
decent.  The number of patches that we moved unreviewed was very small,
which is even better -- hopefully we're not letting too many people
down.

I'm closing this commitfest now.  We even have three weeks before the
next one starts, so everybody can take a break for once!  Yay!

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> I'm closing this commitfest now.  We even have three weeks before the
> next one starts, so everybody can take a break for once!  Yay!

Yay!  And thanks for running this one --- I know it's a mighty
tedious and thankless task.
        regards, tom lane

On 2/8/16 4:19 PM, Alvaro Herrera wrote:
>
> I'm closing this commitfest now.  We even have three weeks before the
> next one starts, so everybody can take a break for once!  Yay!

Many thanks, Álvaro!

By the way, I would be happy to manage the next commitfest.  I've been
watching the process for a while and I think I have a good handle on
what to do. I'm nothing if not organized.

I know this is the last commitfest of the 9.6 development cycle so I'll
understand if the community does not think I have enough experience, but
to my mind it doesn't look all that hard, but rather time-consuming and
tedious.

--
-David
david@pgmasters.net

On 2016/02/09 6:46, Magnus Hagander wrote:
> On Mon, Feb 8, 2016 at 10:19 PM, Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:
> 
>> Hi everybody,
>>
>> I just closed the last few remaining items in the commitfest.  This is
>> the final summary:
>>
>>  Committed: 32.
>>  Moved to next CF: 32.
>>  Rejected: 2.
>>  Returned with Feedback: 33.
>> Total: 99.
>>
>> I think we did a fairly decent job this time around: we only passed a
>> third of the patches to the upcoming commitfest, which seems pretty
>> decent.  The number of patches that we moved unreviewed was very small,
>> which is even better -- hopefully we're not letting too many people
>> down.
>>
>> I'm closing this commitfest now.  We even have three weeks before the
>> next one starts, so everybody can take a break for once!  Yay!
> 
> 
> Thanks for taking on the thankless task of pushing things along!

+1, thank you very much!

Regards,
Amit