Thread
-
InitProcGlobal cleanup
Robert Haas <robertmhaas@gmail.com> — 2011-06-02T17:08:03Z
While working on my patch to reduce the overhead of frequent table locks, I had cause to monkey with InitProcGlobal() and noticed that it's sort of a mess. For reasons that are not clear to me, it allocates one of the three PGPROC arrays using ShemInitStruct() and the other two using ShmemAlloc(). I'm not clear on why we should use different functions for different allocations, and it also seems like it would make sense to do the whole allocation at once instead of doing three separate ones. Also, the setup of AuxiliaryProcs is strangely split into two parts, one at the top of the function (where we allocate the memory) and the other at the bottom (where we initialize it), but there's no clear reason to break it up like that. Any reason not to instead do something like the attached? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: InitProcGlobal cleanup
Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-02T17:53:08Z
Robert Haas <robertmhaas@gmail.com> writes: > While working on my patch to reduce the overhead of frequent table > locks, I had cause to monkey with InitProcGlobal() and noticed that > it's sort of a mess. For reasons that are not clear to me, it > allocates one of the three PGPROC arrays using ShemInitStruct() and > the other two using ShmemAlloc(). I'm not clear on why we should use > different functions for different allocations, and it also seems like > it would make sense to do the whole allocation at once instead of > doing three separate ones. Also, the setup of AuxiliaryProcs is > strangely split into two parts, one at the top of the function (where > we allocate the memory) and the other at the bottom (where we > initialize it), but there's no clear reason to break it up like that. > Any reason not to instead do something like the attached? I find this a whole lot less readable, because you've largely obscured the fact that there are three or four different groups of PGPROC structures being built here and then linked into several different lists/arrays. The code might be okay but it desperately needs more comments. regards, tom lane
-
Re: InitProcGlobal cleanup
Robert Haas <robertmhaas@gmail.com> — 2011-06-02T18:35:31Z
On Thu, Jun 2, 2011 at 1:53 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> While working on my patch to reduce the overhead of frequent table >> locks, I had cause to monkey with InitProcGlobal() and noticed that >> it's sort of a mess. For reasons that are not clear to me, it >> allocates one of the three PGPROC arrays using ShemInitStruct() and >> the other two using ShmemAlloc(). I'm not clear on why we should use >> different functions for different allocations, and it also seems like >> it would make sense to do the whole allocation at once instead of >> doing three separate ones. Also, the setup of AuxiliaryProcs is >> strangely split into two parts, one at the top of the function (where >> we allocate the memory) and the other at the bottom (where we >> initialize it), but there's no clear reason to break it up like that. > >> Any reason not to instead do something like the attached? > > I find this a whole lot less readable, because you've largely obscured > the fact that there are three or four different groups of PGPROC > structures being built here and then linked into several different > lists/arrays. The code might be okay but it desperately needs more > comments. OK, here's a version with more comments. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: InitProcGlobal cleanup
Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-02T19:13:42Z
Robert Haas <robertmhaas@gmail.com> writes: > OK, here's a version with more comments. Looks OK to me, assuming you've checked that the right number of PGPROCs are getting created (in particular the AV launcher is no longer accounted for explicitly). regards, tom lane
-
Re: InitProcGlobal cleanup
Robert Haas <robertmhaas@gmail.com> — 2011-06-02T19:16:28Z
On Thu, Jun 2, 2011 at 3:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> OK, here's a version with more comments. > > Looks OK to me, assuming you've checked that the right number of PGPROCs > are getting created (in particular the AV launcher is no longer > accounted for explicitly). That should be fine, due to the way MaxBackends is initialized. See related comment around guc.c:103. I'll commit this to 9.2 after we branch. (When are we doing that, BTW?) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
9.2 branch and 9.1beta2 timing (was Re: InitProcGlobal cleanup)
Tom Lane <tgl@sss.pgh.pa.us> — 2011-06-02T20:42:56Z
Robert Haas <robertmhaas@gmail.com> writes: > I'll commit this to 9.2 after we branch. (When are we doing that, BTW?) Sometime in the next two weeks I guess ;-). At the PGCon meeting we said 1 June, but seeing that we still have a couple of open beta2 issues I'm not in a hurry. I think a reasonable plan would be to fix the currently known open issues, push out a beta2, and then branch. That would avoid double-patching. We'd want to get this done before the commitfest starts on the 15th, of course, so if we stick to usual release scheduling that would mean wrap next Thursday (June 9), beta2 announce on Monday the 13th, and make the branch somewhere around that date as well. Comments? regards, tom lane
-
Re: 9.2 branch and 9.1beta2 timing (was Re: InitProcGlobal cleanup)
Robert Haas <robertmhaas@gmail.com> — 2011-06-02T21:04:16Z
On Thu, Jun 2, 2011 at 4:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> I'll commit this to 9.2 after we branch. (When are we doing that, BTW?) > > Sometime in the next two weeks I guess ;-). At the PGCon meeting we > said 1 June, but seeing that we still have a couple of open beta2 issues > I'm not in a hurry. > > I think a reasonable plan would be to fix the currently known open > issues, push out a beta2, and then branch. That would avoid > double-patching. We'd want to get this done before the commitfest > starts on the 15th, of course, so if we stick to usual release > scheduling that would mean wrap next Thursday (June 9), beta2 announce > on Monday the 13th, and make the branch somewhere around that date as > well. > > Comments? OK by me. It appears that the open items list is a bit stale: http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Open_Items The first item listed there is, I believe, fixed. I'm not sure about the second. You just volunteered to fix the third, and the fourth is awaiting comments on -bugs. The larger problem is that there are likely some other things that should be listed there, but aren't. If anyone is aware of stuff we need to get done, please add it there... -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
-
Re: 9.2 branch and 9.1beta2 timing (was Re: InitProcGlobal cleanup)
Kevin Grittner <kevin.grittner@wicourts.gov> — 2011-06-02T21:35:26Z
Robert Haas <robertmhaas@gmail.com> wrote: > It appears that the open items list is a bit stale: > > http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Open_Items > > The first item listed there is, I believe, fixed. That was "SSI HOT chain traversal issue" -- which was fixed. I just moved it to "Resolved Issues". > I'm not sure about the second. That's "Make DDL commands SSI-aware". That is two to four lines of code away from complete, but I got stuck last time I tried to figure out where to put those lines. The code for DROP TABLE and TRUNCATE TABLE is not quite as easy to follow as most of the PostgreSQL code base -- or so it seemed to me. I'll take another run at it. A patch will be forthcoming on Sunday at the latest. All other DDL commands are done and have been through some testing. > If anyone is aware of stuff we need to get done, please add it > there... I've submitted a comment-only patch and will be submitting a patch by Sunday to add a few more lines to README-SSI. I don't know that those are beta2 blockers, though. Should I add them to "Not Blockers for Beta2"? -Kevin
-
Re: 9.2 branch and 9.1beta2 timing (was Re: InitProcGlobal cleanup)
Robert Haas <robertmhaas@gmail.com> — 2011-06-02T22:15:24Z
On Thu, Jun 2, 2011 at 5:35 PM, Kevin Grittner <Kevin.Grittner@wicourts.gov> wrote: > Robert Haas <robertmhaas@gmail.com> wrote: > >> It appears that the open items list is a bit stale: >> >> http://wiki.postgresql.org/wiki/PostgreSQL_9.1_Open_Items >> >> The first item listed there is, I believe, fixed. > > That was "SSI HOT chain traversal issue" -- which was fixed. I just > moved it to "Resolved Issues". > >> I'm not sure about the second. > > That's "Make DDL commands SSI-aware". That is two to four lines of > code away from complete, but I got stuck last time I tried to figure > out where to put those lines. The code for DROP TABLE and TRUNCATE > TABLE is not quite as easy to follow as most of the PostgreSQL code > base -- or so it seemed to me. I'll take another run at it. A > patch will be forthcoming on Sunday at the latest. All other DDL > commands are done and have been through some testing. > >> If anyone is aware of stuff we need to get done, please add it >> there... > > I've submitted a comment-only patch and will be submitting a patch > by Sunday to add a few more lines to README-SSI. I don't know that > those are beta2 blockers, though. Should I add them to "Not > Blockers for Beta2"? Either is fine. Comment patches are easy. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company