Обсуждение: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Stefan Kaltenbrunner
Дата:
Tom Lane wrote: > Log Message: > ----------- > Get rid of the need for manual maintenance of the initial contents of > pg_attribute, by having genbki.pl derive the information from the various > catalog header files. This greatly simplifies modification of the > "bootstrapped" catalogs. > > This patch finally kills genbki.sh and Gen_fmgrtab.sh; we now rely entirely on > Perl scripts for those build steps. To avoid creating a Perl build dependency > where there was not one before, the output files generated by these scripts > are now treated as distprep targets, ie, they will be built and shipped in > tarballs. But you will need a reasonably modern Perl (probably at least > 5.6) if you want to build from a CVS pull. this broke the build on spoonbill: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spoonbill&dt=2010-01-05%2015:05:08 manually executing the command shows that the perl process eats more than 250MB of RAM at closely afterwards fails with an out of memory due to hitting the process limit on that box. I don't think that is in any way sane :) # perl -v This is perl, v5.8.8 built for sparc64-openbsd Stefan
On Tue, Jan 5, 2010 at 12:24 PM, Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> wrote: > Tom Lane wrote: >> >> Log Message: >> ----------- >> Get rid of the need for manual maintenance of the initial contents of >> pg_attribute, by having genbki.pl derive the information from the various >> catalog header files. This greatly simplifies modification of the >> "bootstrapped" catalogs. >> >> This patch finally kills genbki.sh and Gen_fmgrtab.sh; we now rely >> entirely on >> Perl scripts for those build steps. To avoid creating a Perl build >> dependency >> where there was not one before, the output files generated by these >> scripts >> are now treated as distprep targets, ie, they will be built and shipped in >> tarballs. But you will need a reasonably modern Perl (probably at least >> 5.6) if you want to build from a CVS pull. > > this broke the build on spoonbill: > > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spoonbill&dt=2010-01-05%2015:05:08 > > manually executing the command shows that the perl process eats more than > 250MB of RAM at closely afterwards fails with an out of memory due to > hitting the process limit on that box. > I don't think that is in any way sane :) > > > # perl -v > This is perl, v5.8.8 built for sparc64-openbsd I just tried this with ulimit -v 131072 and it worked. At 65536 and 32768 it emited an error about being unable to set the locale but still seemed to run. At 32768 it couldn't load all its shared libraries any more so it croaked, but with a different error message. Can we get the output of ulimit -a on that machine? Is there by any chance some other, conflicting Catalog.pm on that machine? ...Robert
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > this broke the build on spoonbill: > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spoonbill&dt=2010-01-05%2015:05:08 > manually executing the command shows that the perl process eats more > than 250MB of RAM at closely afterwards fails with an out of memory due > to hitting the process limit on that box. > I don't think that is in any way sane :) Bizarre. Gen_fmgrtab.pl doesn't eat anywhere near that much RAM here. Anybody else seeing the same? regards, tom lane
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Stefan Kaltenbrunner
Дата:
Robert Haas wrote: > On Tue, Jan 5, 2010 at 12:24 PM, Stefan Kaltenbrunner > <stefan@kaltenbrunner.cc> wrote: >> Tom Lane wrote: >>> Log Message: >>> ----------- >>> Get rid of the need for manual maintenance of the initial contents of >>> pg_attribute, by having genbki.pl derive the information from the various >>> catalog header files. This greatly simplifies modification of the >>> "bootstrapped" catalogs. >>> >>> This patch finally kills genbki.sh and Gen_fmgrtab.sh; we now rely >>> entirely on >>> Perl scripts for those build steps. To avoid creating a Perl build >>> dependency >>> where there was not one before, the output files generated by these >>> scripts >>> are now treated as distprep targets, ie, they will be built and shipped in >>> tarballs. But you will need a reasonably modern Perl (probably at least >>> 5.6) if you want to build from a CVS pull. >> this broke the build on spoonbill: >> >> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=spoonbill&dt=2010-01-05%2015:05:08 >> >> manually executing the command shows that the perl process eats more than >> 250MB of RAM at closely afterwards fails with an out of memory due to >> hitting the process limit on that box. >> I don't think that is in any way sane :) >> >> >> # perl -v >> This is perl, v5.8.8 built for sparc64-openbsd > > I just tried this with ulimit -v 131072 and it worked. At 65536 and > 32768 it emited an error about being unable to set the locale but > still seemed to run. At 32768 it couldn't load all its shared > libraries any more so it croaked, but with a different error message. > > Can we get the output of ulimit -a on that machine? $ ulimit -a time(cpu-seconds) unlimited file(blocks) unlimited coredump(blocks) unlimited data(kbytes) 524288 stack(kbytes) 4096 lockedmem(kbytes) 334589 memory(kbytes) 1000456 nofiles(descriptors) 128 processes 64 > > Is there by any chance some other, conflicting Catalog.pm on that machine? as I said I can reproduce it manually withe the Catalog.pm from the failing build as well. I can succeed building it using the root account but that runs the box more or less out of memory as it eats up to ~550MB RSS and 990MB of SIZE... Stefan
Robert Haas <robertmhaas@gmail.com> writes: > Is there by any chance some other, conflicting Catalog.pm on that machine? Even if there is, shouldn't the use of "perl -I" ensure our version gets loaded? Also, Stefan's log shows that it got through genbki.pl, so whatever the problem is, I don't think it's Catalog.pm's fault. regards, tom lane
On Tue, Jan 5, 2010 at 12:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> Is there by any chance some other, conflicting Catalog.pm on that machine? > > Even if there is, shouldn't the use of "perl -I" ensure our version gets > loaded? I would certainly think so. > Also, Stefan's log shows that it got through genbki.pl, so whatever the > problem is, I don't think it's Catalog.pm's fault. Beats me. I don't have any other ideas at the moment. ...Robert
Robert Haas <robertmhaas@gmail.com> writes: > Beats me. I don't have any other ideas at the moment. Stefan, could you try adding some debugging printouts to the Gen_fmgrtab.pl script to see how far it gets before blowing up? regards, tom lane
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Stefan Kaltenbrunner
Дата:
Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> Beats me. I don't have any other ideas at the moment. > > Stefan, could you try adding some debugging printouts to the > Gen_fmgrtab.pl script to see how far it gets before blowing up? did that and it seems the problem is in the loop that does: foreach my $row (@$data) { # To construct fmgroids.h and fmgrtab.c, we need to inspect some # of the individual data fields. Just splittingon whitespace # won't work, because some quoted fields might contain internal # whitespace. We handle thisby folding them all to a simple # "xxx". Fortunately, this script doesn't need to look at any # fields that mightneed quoting, so this simple hack is # sufficient. ... } it does after around 1050 iterations of that loop at it seems to leak exactly 240kbyte per iteration which sums up to around 250MB in total for the process... Stefan
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > did that and it seems the problem is in the loop that does: > foreach my $row (@$data) > { > # To construct fmgroids.h and fmgrtab.c, we need to inspect some > # of the individual data fields. Just splitting on whitespace Huh. It's weird that I don't see a leak in either 5.8.7 or 5.10.1, which are the two closest perl versions I have handy here. I think this may be a platform-specific Perl bug. Still, it would be nice to work around it if we can. I'm not nearly good enough in Perl to be sure about the semantics of this loop. Is it possible that it's changing the global contents of the @$data structure, rather than just hacking a local copy of each row before pushing some values into @fmgr? regards, tom lane
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Alvaro Herrera
Дата:
Stefan Kaltenbrunner escribió: > foreach my $row (@$data) > { > > # To construct fmgroids.h and fmgrtab.c, we need to inspect some > # of the individual data fields. Just splitting on whitespace > # won't work, because some quoted fields might contain internal > # whitespace. We handle this by folding them all to a simple > # "xxx". Fortunately, this script doesn't need to look at any > # fields that might need quoting, so this simple hack is > # sufficient. > > ... > } > > it does after around 1050 iterations of that loop at it seems to > leak exactly 240kbyte per iteration which sums up to around 250MB in > total for the process... Hmm, is this running some old Perl version? Perhaps it's not freeing memory timely ... maybe unsetting/deleting $row after each iteration? -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Stefan Kaltenbrunner
Дата:
Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: >> did that and it seems the problem is in the loop that does: > >> foreach my $row (@$data) >> { > >> # To construct fmgroids.h and fmgrtab.c, we need to inspect some >> # of the individual data fields. Just splitting on whitespace > > Huh. It's weird that I don't see a leak in either 5.8.7 or 5.10.1, > which are the two closest perl versions I have handy here. I think > this may be a platform-specific Perl bug. Still, it would be nice > to work around it if we can. yeah it is probably some strange platform specific issue - however with some help from the IRC channel I came up with the following patch that considerable reduces the memory bloat (but still needs ~55MB max) and allows the build to succeed. > > I'm not nearly good enough in Perl to be sure about the semantics > of this loop. Is it possible that it's changing the global contents > of the @$data structure, rather than just hacking a local copy of > each row before pushing some values into @fmgr? hmm it is leaking a constant amount of 240kbyte per loop iteration with the original code but yeah very weird issue... Stefan Index: src/backend/utils/Gen_fmgrtab.pl =================================================================== RCS file: /projects/cvsroot/pgsql/src/backend/utils/Gen_fmgrtab.pl,v retrieving revision 1.4 diff -u -r1.4 Gen_fmgrtab.pl --- src/backend/utils/Gen_fmgrtab.pl 5 Jan 2010 01:06:56 -0000 1.4 +++ src/backend/utils/Gen_fmgrtab.pl 5 Jan 2010 19:14:10 -0000 @@ -56,7 +56,7 @@ } my $data = $catalogs->{pg_proc}->{data}; -foreach my $row (@$data) +while( my $row = shift @$data ) { # To construct fmgroids.h and fmgrtab.c, we need to inspect some
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > yeah it is probably some strange platform specific issue - however with > some help from the IRC channel I came up with the following patch that > considerable reduces the memory bloat (but still needs ~55MB max) and > allows the build to succeed. What about Alvaro's idea of adding $row = undef; at the bottom of the loop? That seems marginally less ugly... regards, tom lane
Garick Hamlin <ghamlin@isc.upenn.edu> writes: > On Tue, Jan 05, 2010 at 02:02:51PM -0500, Tom Lane wrote: >> I'm not nearly good enough in Perl to be sure about the semantics >> of this loop. Is it possible that it's changing the global contents >> of the @$data structure, rather than just hacking a local copy of >> each row before pushing some values into @fmgr? > Yes, that is what would happen. > Is that a problem? (I haven't read the script) Well, it *shouldn't* be a problem, but what it looks like to me is that Stefan's perl version is somehow leaking one copy of the whole $data structure each time through the loop. If we were to copy and then modify each row individually, maybe that'd dodge the bug. regards, tom lane
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Garick Hamlin
Дата:
On Tue, Jan 05, 2010 at 02:02:51PM -0500, Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: > > did that and it seems the problem is in the loop that does: > > > foreach my $row (@$data) > > { > > > # To construct fmgroids.h and fmgrtab.c, we need to inspect some > > # of the individual data fields. Just splitting on whitespace > > Huh. It's weird that I don't see a leak in either 5.8.7 or 5.10.1, > which are the two closest perl versions I have handy here. I think > this may be a platform-specific Perl bug. Still, it would be nice > to work around it if we can. > > I'm not nearly good enough in Perl to be sure about the semantics > of this loop. Is it possible that it's changing the global contents > of the @$data structure, rather than just hacking a local copy of > each row before pushing some values into @fmgr? Yes, that is what would happen. I have not read this script at all but that is how perl works... $row is an aliased to each scalar in the list '(@$data)' There is no copying. Copying happens on list assignment, but not with for/foreach. If you wanted a copy you need something like: foreach my $row (@{[ @$data ]}) { for a shallow copy. Is that a problem? (I haven't read the script) Garick > > regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Stefan Kaltenbrunner
Дата:
Tom Lane wrote: > Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: >> yeah it is probably some strange platform specific issue - however with >> some help from the IRC channel I came up with the following patch that >> considerable reduces the memory bloat (but still needs ~55MB max) and >> allows the build to succeed. > > What about Alvaro's idea of adding > > $row = undef; > > at the bottom of the loop? That seems marginally less ugly... not sure I want to argue about the ugliness of perl but that has the same effect as my patch so either way would do to get spoonbill green again. Stefan
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes: >> What about Alvaro's idea of adding >> $row = undef; >> at the bottom of the loop? That seems marginally less ugly... > not sure I want to argue about the ugliness of perl but that has the > same effect as my patch so either way would do to get spoonbill green again. OK, done. regards, tom lane
Re: Re: [COMMITTERS] pgsql: Get rid of the need for manual maintenance of the initial
От
Andrew Dunstan
Дата:
Tom Lane wrote: > I'm not nearly good enough in Perl to be sure about the semantics > of this loop. Is it possible that it's changing the global contents > of the @$data structure, rather than just hacking a local copy of > each row before pushing some values into @fmgr? > That's exactly what it does. The loop variable is an alias. See perlsyn(1) for details. These two lines appear to be suspicious: $row->{bki_values} =~ s/"[^"]*"/"xxx"/g; @{$row}{@attnames} = split /\s+/, $row->{bki_values}; Something like: (my $bkival = $row->{bki_values}) =~ s/"[^"]*"/"xxx"/g; my $atts = {}; @{$atts}{@attnames} = split /\s+/, $bkival; might work better. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > Something like: > (my $bkival = $row->{bki_values}) =~ s/"[^"]*"/"xxx"/g; > my $atts = {}; > @{$atts}{@attnames} = split /\s+/, $bkival; > might work better. I committed Alvaro's suggestion (undef at the bottom), but feel free to clean it up if you like this way better. regards, tom lane