Thread: CommitFest 2009-11: One week update
There were 32 patches actively involved in the standard review process for this CommitFest, ignoring the ones with an assigned committer who is also reviewing and ones that were done earlier. Here's where we're at with those: Commited: 5 Returned with feedback: 2 Waiting for author: 12 Needs review: 13 That puts us a little over half done on the review side, with maybe 1/5 of the things that might get committed completely finished. We have no reviewer assigned to following patches yet: SE-PostgreSQL/Lite DTrace Memory management probes DTrace SLRU and executor probes Listen / Notify rewrite That last one seems to have settled down enough to be worth taking a dive into now, if anyone done with their initial assignments wants a small patch to consider for their second round. As far as I know we're waiting for the following reviews still: Largeobject access controls (Abhijit Menon-Sen) LDAP search during authentication (Magnus Hagander) XLogInsert: move some work out of critical section (Stephen Frost) EXPLAIN BUFFERS (Euler Taveira de Oliveira) information_schema performance (Peter Eisentraut) New VACUUM FULL (Jeff Davis) per-tablespace random_page_cost/seq_page_cost (Bernd Helmle) pgbench setshell command (Greg Smith) PL/Python array support (Josh Tolley) Application Name (Andres Freund) I'm not sure whether next week's holiday is going to help or hurt the review schedule. I plan a rare round of being AFK during that time. I hope to do another round of prodding everyone before the break, and I'll re-summarize where we're then at around a week from now. -- Greg Smith 2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support greg@2ndQuadrant.com www.2ndQuadrant.com
I think my patch has been adequately reviewed and that I am not likely to have time to revise it in time for this CommitFest. ...Robert On Nov 22, 2009, at 1:53 AM, Greg Smith <greg@2ndquadrant.com> wrote: > There were 32 patches actively involved in the standard review > process for this CommitFest, ignoring the ones with an assigned > committer who is also reviewing and ones that were done earlier. > Here's where we're at with those: > > Commited: 5 > Returned with feedback: 2 > Waiting for author: 12 > Needs review: 13 > > That puts us a little over half done on the review side, with maybe > 1/5 of the things that might get committed completely finished. > > We have no reviewer assigned to following patches yet: > > SE-PostgreSQL/Lite > DTrace Memory management probes > DTrace SLRU and executor probes > Listen / Notify rewrite > > That last one seems to have settled down enough to be worth taking a > dive into now, if anyone done with their initial assignments wants a > small patch to consider for their second round. > > As far as I know we're waiting for the following reviews still: > > Largeobject access controls (Abhijit Menon-Sen) > LDAP search during authentication (Magnus Hagander) > XLogInsert: move some work out of critical section (Stephen Frost) > EXPLAIN BUFFERS (Euler Taveira de Oliveira) > information_schema performance (Peter Eisentraut) > New VACUUM FULL (Jeff Davis) > per-tablespace random_page_cost/seq_page_cost (Bernd Helmle) > pgbench setshell command (Greg Smith) > PL/Python array support (Josh Tolley) > Application Name (Andres Freund) > > I'm not sure whether next week's holiday is going to help or hurt > the review schedule. I plan a rare round of being AFK during that > time. I hope to do another round of prodding everyone before the > break, and I'll re-summarize where we're then at around a week from > now. > > -- > Greg Smith 2ndQuadrant Baltimore, MD > PostgreSQL Training, Services and Support > greg@2ndQuadrant.com www.2ndQuadrant.com > > > -- > Sent via pgsql-rrreviewers mailing list (pgsql-rrreviewers@postgresql.org > ) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-rrreviewers
On Sun, Nov 22, 2009 at 01:53:33AM -0500, Greg Smith wrote: > PL/Python array support (Josh Tolley) For whatever it's worth, I've not forgotten my assignment, and plan to get an initial review done this week. -- Joshua Tolley / eggyknap End Point Corporation http://www.endpoint.com
Attachment
--On 22. November 2009 15:45:23 -0500 Robert Haas <robertmhaas@gmail.com> wrote: > I think my patch has been adequately reviewed and that I am not likely to > have time to revise it in time for this CommitFest. Yeah, i've just emailed Robert wether he's going to work on this during this CommitFest but i was very busy at work and forgot to update the status. Thanks for cleaning this up.
Now that we've cleared the holiday and gotten a few more reviews in, wanted to summarize where we're at again and remind everyone we're waiting on of that fact. The pace of reviews is holding up the rest of the CommitFest progress now, and there's not that much work left to do be done. One more good push and this could be over from the reviewer side. We still have no reviewer assigned to following patches yet: SE-PostgreSQL/Lite DTrace Memory management probes DTrace SLRU and executor probes SE-PostgreSQL/Lite is at least getting some discussion on the lists now. If any of the people who were suggesting they couldn't help out until around now can handle DTrace patches, looking at one or both of those would be helpful. The following patches have a reviewer who's not a committer, but we haven't heard anything from them yet: Largeobject access controls (Abhijit Menon-Sen) XLogInsert: move some work out of critical section (Stephen Frost) EXPLAIN BUFFERS (Euler Taveira de Oliveira) Since the last of those two are being reviewed by committers I'm not worried about them. As for Abhijit, Stephen, and Euler: can I get a confirmation whether you're working on the review? I think we can get other people to look at these if you just can't find the time. Another problem right now are patches where a review was done, the patch updated, and now a re-review is needed. I've been trying to not give anyone a hard time over these, as in many cases the initial review was done quite quickly, but they're starting to pile up now: tsearch parser inefficiency with urls or emails. Kevin Grittner ProcessUtility_hook Euler Taveira de Oliveira Operator Exclusion Constraints Robert Haas Syntax for partitioning Marko Tiikkaja Aggregate ORDER BY support Hitoshi Harada add more frame types in window functions (ROWS) Andrew Gierth Again, if anyone on this list had time before but doesn't now, please just let me know. Getting a second person for the re-review is a good thing for that patch if we can marshal the resources, and if it's still going to be a while before you can revisit the patch yourself that may be what we need to do. -- Greg Smith 2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support greg@2ndQuadrant.com www.2ndQuadrant.com
>>>>> "Greg" == Greg Smith <greg@2ndquadrant.com> writes: Greg> Another problem right now are patches where a review was done, the Greg> patch updated, and now a re-review is needed. I've been trying to not Greg> give anyone a hard time over these, as in many cases the initial Greg> review was done quite quickly, but they're starting to pile up now: Greg> add more frame types in window functions (ROWS) Andrew Gierth I was hoping that there would have been some feedback on the issue that Hitoshi raised on -hackers some days ago, since it seems to be the only outstanding issue with this patch now. Since this is a question about stability of an API which is potentially in use by third-party modules (I don't think we have any easy way to know if it's _actually_ in use), I'm not happy with making an accept/reject decision based on it without feedback from others. I have now posted to -hackers in an attempt to prompt some response. -- Andrew.
Greg Smith wrote: > The following patches have a reviewer who's not a committer, but we > haven't heard anything from them yet: > > Largeobject access controls (Abhijit Menon-Sen) It looks like Abhijit is having some trouble right now, just pulled him off this assigment. Anyone else interested in reviewing this one? It's now on the list of patches completely missing a reviewer. -- Greg Smith 2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support greg@2ndQuadrant.com www.2ndQuadrant.com
--On 2. Dezember 2009 05:02:18 -0500 Greg Smith <greg@2ndquadrant.com>
wrote:
> t looks like Abhijit is having some trouble right now, just pulled him
> off this assigment. Anyone else interested in reviewing this one? It's
> now on the list of patches completely missing a reviewer.
I can have a first look on this, if that helps. There wasn't plenty of work
for me in this round...
Bernd
On Wed, Dec 2, 2009 at 2:33 AM, Greg Smith <greg@2ndquadrant.com> wrote: > > If any of the people who were suggesting they couldn't help out until around > now can handle DTrace patches, looking at one or both of those would be > helpful. > i have no access to dtrace at all... but now i can take a patch, maybe another one of those that you mention? -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
--On 2. Dezember 2009 15:26:19 -0500 Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:
> i have no access to dtrace at all... but now i can take a patch, maybe
> another one of those that you mention?
Hmm i'm running OS X...i already proposed Greg to take my hands on the
Largeobject Access Control Patch, but now it seems better to look at those
DTrace things. I originally hesitated to review those since i'm not very
familiar with DTrace, but if it helps (and i can improve my skills here),
i'm fine with it and you can review the Largeobject patches?
Bernd
On Wed, Dec 2, 2009 at 4:13 PM, Bernd Helmle <bernd@oopsware.de> wrote: > > > --On 2. Dezember 2009 15:26:19 -0500 Jaime Casanova > <jcasanov@systemguards.com.ec> wrote: > >> i have no access to dtrace at all... but now i can take a patch, maybe >> another one of those that you mention? > > > Hmm i'm running OS X...i already proposed Greg to take my hands on the > Largeobject Access Control Patch, but now it seems better to look at those > DTrace things. I originally hesitated to review those since i'm not very > familiar with DTrace, but if it helps (and i can improve my skills here), > i'm fine with it and you can review the Largeobject patches? > that's fine with me... -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157
Jaime Casanova wrote: > On Wed, Dec 2, 2009 at 4:13 PM, Bernd Helmle <bernd@oopsware.de> wrote: > >> Hmm i'm running OS X...i already proposed Greg to take my hands on the >> Largeobject Access Control Patch, but now it seems better to look at those >> DTrace things. I originally hesitated to review those since i'm not very >> familiar with DTrace, but if it helps (and i can improve my skills here), >> i'm fine with it and you can review the Largeobject patches? >> >> > that's fine with me... > CommitFest page updated as such. Bernd, I'd suggest starting with the "SLRU and executor probes" patch, as that one is a lot less controversial than the other. Jaime, I just got a note from Abhijit suggesting he is still working on the review of the Largeobject patch; it's complicated enough that having two people look at it isn't a bad idea though if you want to (or already have started) dig into anyway. -- Greg Smith 2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support greg@2ndQuadrant.com www.2ndQuadrant.com
Jaime Casanova wrote: > On Wed, Dec 2, 2009 at 4:13 PM, Bernd Helmle <bernd@oopsware.de> wrote: >> >> --On 2. Dezember 2009 15:26:19 -0500 Jaime Casanova >> <jcasanov@systemguards.com.ec> wrote: >> >>> i have no access to dtrace at all... but now i can take a patch, maybe >>> another one of those that you mention? >> >> Hmm i'm running OS X...i already proposed Greg to take my hands on the >> Largeobject Access Control Patch, but now it seems better to look at those >> DTrace things. I originally hesitated to review those since i'm not very >> familiar with DTrace, but if it helps (and i can improve my skills here), >> i'm fine with it and you can review the Largeobject patches? >> > > that's fine with me... Jaime, Thanks for your volunteering. Since you reviewed this patch at the last commit fest, I think it is not difficult to check the code. It was just a revised version according to the comments from Tom Lane. Greg referenced my previous post which introduced this feature. http://archives.postgresql.org/message-id/4A9757F6.3010401@ak.jp.nec.com http://archives.postgresql.org/message-id/4A9B5AD1.3090002@ak.jp.nec.com Then, this patch is moved to "ready for commiter", but Tom Lane commented some of his complaints, as follows: http://archives.postgresql.org/message-id/15563.1254845821@sss.pgh.pa.us http://archives.postgresql.org/message-id/3224.1255488185@sss.pgh.pa.us The biggest issue was incompatibility of snapshot on references to the pg_largeobject_metadata and pg_largeobject system catalogs. In the previous revision, it always refered the new pg_largeobject_metadata catalog with SnapshotNow, but inv_api.c may refer the pg_largeobject with query's snapshot in read-only mode, not always SnapshotNow. It meaned we could see unvisible changes in the metadata which contains access privileges, if we apply incompatible snapshot between metadata and contents. So, I added pg_largeobject_aclcheck_snapshot() which takes an argument to give a snapthot to be used to scan pg_largeobeject_metadata. It is invoked from inv_api.c, and the caller delivers an appropriate snapshot (it may be SnapshotNow, or not). Anyway, the point is that we should apply an identical snapshot to scan both of data(pg_largeobject) and metadata(pg_largeobject_metadata). The _snapshot() interface seems to you a bit strange, but it has a reason. :-) In addition, the following points are changed in new revision: - Syscache support has gone, because it can consume unexpected amount of local memory when user tries to create massive number of large objects. - ac_largeobject_*() has gone, because "Reworks for Access Control" patch was rejected in the lasst commit fest. - pg_dump support was added. - \lo_list support multiple server version, not only v8.5 or later. - documentations and user visible error messages were fixed. (I surprised at "largeobject" is an incorrect English word. :-) Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai@ak.jp.nec.com>