Thread: September 2012 commitfest
People, The commitfest currently in progress seems to have ground to a halt. We really need to get things moving, because we're three weeks onto it and there is a large number of patches still outstanding. Some numbers: we got 65 patches this time, of which we rejected 4 and returned 3 with feedback. 14 patches have already been committed, and 13 are waiting on their respective authors. 25 patches need review, and 6 are said to be ready for committers. Many of those patches waiting on authors have been in such state for a rather long time. I feel inclined to mark them "returned with feedback", and have them posted again for the next commitfest. Patches in "Ready for committer" state: * Array ELEMENT Foreign Keys * Updatable views * plpgsql_check_function * parallel pg_dump * embedded list interface * Move postgresql_fdw_validator into dblink Some of the patches needing review where booted from the previous commitfest. These were said to have higher reviewing priority during this commitfest. * FOR KEY SHARE foreign keys * Trim trailing NULL columns * Skip checkpoint on promoting from streaming replication * Row-Level Security * pg_stat_lwlocks view - lwlocks statistics * New statistics for WAL buffer dirty writes * support INSERT INTO...RETURNING with partitioned table using rule * Reworks for generic ALTER commands (Sorry if I missed some). Please help in whatever capacity you can. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> The commitfest currently in progress seems to have ground to a halt.
Indeed :-(. I plead guilty as much as anybody else to not making it
a high priority. But do we even have anyone acting as commitfest
manager? Periodic nagging seems to be necessary to move things forward.
> Patches in "Ready for committer" state:
> * Array ELEMENT Foreign Keys
> * Updatable views
> * plpgsql_check_function
> * parallel pg_dump
> * embedded list interface
> * Move postgresql_fdw_validator into dblink
I have fairly strong opinions about the first three, and will endeavor
to work on them next.
IIRC, the parallel pg_dump one is said to need review by a Windows
expert, which is not me, so I've not looked at it.
I'm not sure that the embedded list one can honestly be said to be ready
for commit considering the lack of consensus on it, but we seem to be
making some progress anyway.
I'm not sure if the validator change is noncontroversial or not ---
I seem to recall that Peter and I had different opinions about how to do
that.
regards, tom lane
Tom Lane wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > The commitfest currently in progress seems to have ground to a halt. > > Indeed :-(. I plead guilty as much as anybody else to not making it > a high priority. But do we even have anyone acting as commitfest > manager? Periodic nagging seems to be necessary to move things forward. Yeah. Apparently we don't so this is my first attempt. > > Patches in "Ready for committer" state: > > * Array ELEMENT Foreign Keys > > * Updatable views > > * plpgsql_check_function > > * parallel pg_dump > > * embedded list interface > > * Move postgresql_fdw_validator into dblink > > I have fairly strong opinions about the first three, and will endeavor > to work on them next. Excellent, thanks. > IIRC, the parallel pg_dump one is said to need review by a Windows > expert, which is not me, so I've not looked at it. Andrew? Magnus? > I'm not sure that the embedded list one can honestly be said to be ready > for commit considering the lack of consensus on it, but we seem to be > making some progress anyway. I posted an updated version this morning. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
I wrote: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: >> * Move postgresql_fdw_validator into dblink > I'm not sure if the validator change is noncontroversial or not --- > I seem to recall that Peter and I had different opinions about how to do > that. On rechecking the archives, it looks like Peter now agrees with removing the existing postgresql_fdw_validator and creating a new one in dblink: http://archives.postgresql.org/pgsql-hackers/2012-07/msg00629.php So I think everyone is on the same page about this now. I see a couple of cosmetic things I don't like about the latest version of that patch, but I'll fix them and commit. regards, tom lane
On Wed, Oct 10, 2012 at 12:19:17PM -0300, Alvaro Herrera wrote: > Many of those patches waiting on authors have been in such state for a > rather long time. I feel inclined to mark them "returned with > feedback", and have them posted again for the next commitfest. +1
On Wed, Oct 10, 2012 at 6:17 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Tom Lane wrote: >> Alvaro Herrera <alvherre@2ndquadrant.com> writes: >> IIRC, the parallel pg_dump one is said to need review by a Windows >> expert, which is not me, so I've not looked at it. > > Andrew? Magnus? There's, unfortunately, not a chance I'll have a time to look at any of that until after pgconf.eu. Sorry. -- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/
On 10/11/2012 02:22 PM, Magnus Hagander wrote: > On Wed, Oct 10, 2012 at 6:17 PM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: >> Tom Lane wrote: >>> Alvaro Herrera <alvherre@2ndquadrant.com> writes: >>> IIRC, the parallel pg_dump one is said to need review by a Windows >>> expert, which is not me, so I've not looked at it. >> Andrew? Magnus? > There's, unfortunately, not a chance I'll have a time to look at any > of that until after pgconf.eu. Sorry. I have a quietish few days starting on Saturday, will be looking at this then. Is it only the Windows aspect that needs reviewing? Are we more or less happy with the rest? cheers andrew
On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > I have a quietish few days starting on Saturday, will be looking at this > then. Is it only the Windows aspect that needs reviewing? Are we more or > less happy with the rest? I think the Windows issues were the biggest thing, but I suspect there may be a few other warts as well. It's a lot of code, and it's modifying pg_dump, which is an absolute guarantee that it's built on a foundation made out of pure horse manure. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 11 October 2012 20:30, Robert Haas <robertmhaas@gmail.com> wrote: > On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >> I have a quietish few days starting on Saturday, will be looking at this >> then. Is it only the Windows aspect that needs reviewing? Are we more or >> less happy with the rest? > > I think the Windows issues were the biggest thing, but I suspect there > may be a few other warts as well. It's a lot of code, and it's > modifying pg_dump, which is an absolute guarantee that it's built on a > foundation made out of pure horse manure. That may be so, but enough people dependent upon it that now I'm wondering whether we should be looking to create a new utility altogether, or at least have pg_dump_parallel and pg_dump to avoid any screw ups with people's backups/restores. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Oct 11, 2012 at 3:37 PM, Simon Riggs <simon@2ndquadrant.com> wrote: > On 11 October 2012 20:30, Robert Haas <robertmhaas@gmail.com> wrote: >> On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>> I have a quietish few days starting on Saturday, will be looking at this >>> then. Is it only the Windows aspect that needs reviewing? Are we more or >>> less happy with the rest? >> >> I think the Windows issues were the biggest thing, but I suspect there >> may be a few other warts as well. It's a lot of code, and it's >> modifying pg_dump, which is an absolute guarantee that it's built on a >> foundation made out of pure horse manure. > > That may be so, but enough people dependent upon it that now I'm > wondering whether we should be looking to create a new utility > altogether, or at least have pg_dump_parallel and pg_dump to avoid any > screw ups with people's backups/restores. Well, I think pg_dump may well need a full rewrite to be anything like sane. But I'm not too keen about forking it as part of adding parallel dump. I think we can sanely hack this patch into what's there now. It's liable to be a bit hard to verify, but in the long run having two copies of the code is going to be a huge maintenance headache, so we should avoid that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/12/2012 03:07 PM, Robert Haas wrote: > On Thu, Oct 11, 2012 at 3:37 PM, Simon Riggs <simon@2ndquadrant.com> wrote: >> On 11 October 2012 20:30, Robert Haas <robertmhaas@gmail.com> wrote: >>> On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>>> I have a quietish few days starting on Saturday, will be looking at this >>>> then. Is it only the Windows aspect that needs reviewing? Are we more or >>>> less happy with the rest? >>> I think the Windows issues were the biggest thing, but I suspect there >>> may be a few other warts as well. It's a lot of code, and it's >>> modifying pg_dump, which is an absolute guarantee that it's built on a >>> foundation made out of pure horse manure. >> That may be so, but enough people dependent upon it that now I'm >> wondering whether we should be looking to create a new utility >> altogether, or at least have pg_dump_parallel and pg_dump to avoid any >> screw ups with people's backups/restores. > Well, I think pg_dump may well need a full rewrite to be anything like > sane. But I'm not too keen about forking it as part of adding > parallel dump. I think we can sanely hack this patch into what's > there now. It's liable to be a bit hard to verify, but in the long > run having two copies of the code is going to be a huge maintenance > headache, so we should avoid that. > That's my feeling too. cheers andrew
On 12 October 2012 20:07, Robert Haas <robertmhaas@gmail.com> wrote: > On Thu, Oct 11, 2012 at 3:37 PM, Simon Riggs <simon@2ndquadrant.com> wrote: >> On 11 October 2012 20:30, Robert Haas <robertmhaas@gmail.com> wrote: >>> On Thu, Oct 11, 2012 at 2:42 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>>> I have a quietish few days starting on Saturday, will be looking at this >>>> then. Is it only the Windows aspect that needs reviewing? Are we more or >>>> less happy with the rest? >>> >>> I think the Windows issues were the biggest thing, but I suspect there >>> may be a few other warts as well. It's a lot of code, and it's >>> modifying pg_dump, which is an absolute guarantee that it's built on a >>> foundation made out of pure horse manure. >> >> That may be so, but enough people dependent upon it that now I'm >> wondering whether we should be looking to create a new utility >> altogether, or at least have pg_dump_parallel and pg_dump to avoid any >> screw ups with people's backups/restores. > > Well, I think pg_dump may well need a full rewrite to be anything like > sane. But I'm not too keen about forking it as part of adding > parallel dump. I think we can sanely hack this patch into what's > there now. It's liable to be a bit hard to verify, but in the long > run having two copies of the code is going to be a huge maintenance > headache, so we should avoid that. I agree that maintaining 2 copies of the code would be a huge maintenance headache in the long run. I wasn't suggesting we do it for more than 1 release, after which we just have 2 names for same program. I think the phrase "a bit hard to verify" probably isn't good in conjunction with backup utilities, where stability is preferred. I'm not wedded to my suggestion of how we handle the risk of it going wrong, but if we see a risk then we should do something about it. -- Simon Riggs http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
A week ago, I wrote: > Some numbers: we got 65 patches this time, of which we rejected 4 and > returned 3 with feedback. 14 patches have already been committed, and > 13 are waiting on their respective authors. 25 patches need review, and > 6 are said to be ready for committers. A week later, numbers have changed but not by much. We now have 66 patches (one patch from the previous commitfest was supposed to be moved to this one but didn't). Rejected patches are still 4; there are now 7 returned with feedback. 17 are committed, and 10 are waiting on authors. 21 patches need review. Most of the remaining Waiting-on-author patches have seen recent activity, which is why I didn't close them as returned. Authors should speak up soon -- there is no strict policy but I don't think I'm going to wait later than this Friday for some final version to be submitted that can be considered ready for committer. If more work is needed than simple fixes, authors are encouraged to close such patches as "returned with feedback" themselves and resubmit during the next commitfest. Most worrying to me are the large number of patches waiting for a review -- in some cases they are waiting even for an initial review. Here's a list: Server Features * Timeout framework extension and lock_timeout * Patch to compute Max LSN of Data Pages * FOR KEY SHARE foreign keys * [PoC] Writable Foreign Tables * Incorrect behaviour when using a GiST index on points Performance * Skip checkpoint on promoting from streaming replication * Decrease GiST bloat when penalties are identical * Range Types statistics * Performance Improvement by reducing WAL for Update Operation * Adjacent in SP-GiST for range-types * 2d-mapping based GiST for ranges * Performance Improvement in Buffer Management for Select operation Security * Row-Level Security * Extend argument of OAT_POST_CREATE System Administration * New statistics for WAL buffer dirty writes * Switching timeline over streaming replication Miscellaneous * support INSERT INTO...RETURNING with partitioned table using rule * Reworks for generic ALTER commands * copy result to psql variables * FDW for PostgreSQL * pgbench - custom logging step, estimate of remaining time -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Wednesday, October 17, 2012 9:38 PM Alvaro Herrera wrote: > A week ago, I wrote: > > > Some numbers: we got 65 patches this time, of which we rejected 4 and > > returned 3 with feedback. 14 patches have already been committed, and > > 13 are waiting on their respective authors. 25 patches need review, > and > > 6 are said to be ready for committers. > > A week later, numbers have changed but not by much. We now have 66 > patches (one patch from the previous commitfest was supposed to be moved > to this one but didn't). Rejected patches are still 4; there are now 7 > returned with feedback. 17 are committed, and 10 are waiting on > authors. 21 patches need review. > > Most of the remaining Waiting-on-author patches have seen recent > activity, which is why I didn't close them as returned. Authors should > speak up soon -- there is no strict policy but I don't think I'm going > to wait later than this Friday for some final version to be submitted > that can be considered ready for committer. For the Patch, Trim trailing NULL columns, I have provided the performance data required and completed the review. There are only few review comments which can be addressed. So is it possible that I complete them and mark it as "Ready For Committer" or what else can be the way to proceed for this patch if author doesn't respond. With Regards, Amit Kapila.
Amit Kapila wrote: > For the Patch, Trim trailing NULL columns, I have provided the performance > data required > and completed the review. There are only few review comments which can be > addressed. > So is it possible that I complete them and mark it as "Ready For Committer" > or what else can be the way to proceed for this patch > if author doesn't respond. Sure, you can do that. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Thursday, October 18, 2012 7:55 PM Alvaro Herrera wrote: > Amit Kapila wrote: > > > For the Patch, Trim trailing NULL columns, I have provided the > performance > > data required > > and completed the review. There are only few review comments which can > be > > addressed. > > So is it possible that I complete them and mark it as "Ready For > Committer" > > or what else can be the way to proceed for this patch > > if author doesn't respond. > > Sure, you can do that. Done. With Regards, Amit Kapila.
The September 2012 commitfest has been running now for six weeks, and
we've been making some progress. That stalled a bit last week due to
the European conference; hopefully there are now some renewed energies
to let us finish it up soon.
One thing I do *not* want to do is send many patches to the next
commitfest. That will only make the final commitfests worse by piling
up stuff no one seems to care about (except their submitter). So if
you're able to help, please do.
There are now 65 patches listed; 20 of them have been committed so far,
and 22 have been returned with feedback. 1 was sent to the next
commitfest ("lock_timeout").
Waiting on Author: 1
Needs Review: 10
Ready for Committer: 7
One patch is "Waiting on Author" ("Incorrect behaviour when using a GiST
index on points"). This is a bug fix and so requires special
dedication, and backpatch. One of the GiST hackers should really get to
it soon, please.
Ten patches are in "Needs Review" state.
- From Amit Kapila:
* Performance Improvement in Buffer Management for Select operation Jeff Janes has been looking at this. I think we
shouldclose it for now. More performance figures are needed.
* Patch to compute Max LSN of Data Pages Apparently it's difficult to get people excited about this. There was no
discussion.
- From Alexander Korotkov:
* Decrease GiST bloat when penalties are identical
* Range Types statistics
* Adjacent in SP-GiST for range-types
I think these three patches are pretty close to ready, according to the
review comments. If somebody can give them a final look to commit this
week, that'd be great. May need a final revision to be sent.
* 2d-mapping based GiST for ranges
This one doesn't seem to have gotten any review. I would like to send
this one to the next commitfest.
- Other guys:
* FOR KEY SHARE foreign keys (me) This one is seeing some more discussion lately. I'm not closing it just yet mainly
toavoid killing the discussion.
* [PoC] Writable Foreign Tables (KaiGai Kohei) This needs to be examined by someone with executor insight.
* Skip checkpoint on promoting from streaming replication (Kyotaro Horiguchi) Simon is going to handle this at some
point;I will punt to next CF.
Additionally, seven patches are "Ready for Committer":
* Updatable views Tom said he was going to handle this one
* tuplesort memory usage: grow_memtuples Greg Stark signed up for this
* Trim trailing NULL columns Josh Berkus was going to do performance testing, but if he published anything I can't find
it. Robert said, in the previous commitfest, that if the benchmarks were right then "this patch is ready to go in".
It'sbeen long since any committer weighed in on this thread, though.
* Make pg_basebackup configure and start standby Magnus signed up for this
* Fix console prompt encoding on Windows Noah thinks this is pretty much ready to commit. No one signed up, but a
Windowscommitter needed. Magnus, Andrew?
* parallel pg_dump Andrew Dunstan said he was going to handle this.
* plpgsql_check_function Selena submitted an re-indented copy of the last one submitted by Pavel but apparently
somethingwas wrong about what she did; got advice about it but didn't get around to submitting another version. There
wasanother thread around message 4F7C7346.2090407@enterprisedb.com and it doesn't look like Selena followed that one.
I'mnot sure where this patch stands, really; the history needs to be checked carefully to ensure what Pavel last
submittedis in line with the review in the previous discussions.
There are 163 emails in three threads, each starting at
CAFj8pRDkkzSi611Eimp=AXj2HD46k-W46GDvW9MKAD2OgwoKag@mail.gmail.com 1330807185-sup-2696@alvh.no-ip.org
CAFj8pRAYVTQYCL8_NF_hDQjc0m+JBvbwR6E_ZJ0SJfkKQ9m2kA@mail.gmail.com
I vaguely recall Greg Stark signed up for another patch recently, but I
can't readily find which one it was.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Mon, Oct 29, 2012 at 3:27 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > * tuplesort memory usage: grow_memtuples > Greg Stark signed up for this I'll commit this later this week. I looked at it briefly at the conference but I think it actually does need some minor tweaks. > * Trim trailing NULL columns > Josh Berkus was going to do performance testing, but if he published > anything I can't find it. Robert said, in the previous commitfest, > that if the benchmarks were right then "this patch is ready to go in". > It's been long since any committer weighed in on this thread, though. > > I vaguely recall Greg Stark signed up for another patch recently, but I > can't readily find which one it was. In the commitfest app I put my name down on the trim trailing NULL columns patch. I'm generally for the idea and the benchmarks looked convincing so unless there's something obviously broken I'll probably commit it more or less as-is. -- greg
I said last week: > Waiting on Author: 1 > Needs Review: 10 > Ready for Committer: 7 Now there are only 9 patches "Ready for committer". All other patches have either been moved to the next commitfest, or returned with feedback. So we've made some progress, but we need a final push from committers. A few of these patches have had a committer said they would look onto them, so I've left them in the September commitfest in case one of them has time to look onto them this week. Oleg, Teodor:Incorrect behaviour when using a GiST index on points* This is a bug fix. Greg Stark:tuplesort memory usage: grow_memtuplesTrim trailing NULL columns Tom Lane:Updatable views Magnus:Make pg_basebackup configure and start standby Andrew Dunstan:parallel pg_dump Heikki?Decrease GiST bloat when penalties are identical Magnus? Andrew?Fix console prompt encoding on Windows ?plpgsql_check_function -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services