Re: [HACKERS] 8.3 Release Schedule
Tom Lane wrote: Joshua D. Drake [EMAIL PROTECTED] writes: Dave Page wrote: Actually thinking about it, I think we should plan the next cycle based on whatever ends up happening this time - eg. April freeze, Aug-Sept beta, Oct release. I actually would be more inclined to have an even shorter cycle release next time... e.g. January Freeze. The original idea was sound, make it so we aren't testing in the middle of summer. I think part of the problem is exactly that the freeze period has stretched into summer, and so people aren't around for one reason or another, and so it's going slower than one could wish. As already noted, when we set the schedule we were not expecting to have so many large patches dropped on us at the very end of the devel cycle. What I'd like to think about is how can we avoid *that* happening again? Maybe there's no way, because human nature is to not finish stuff much before the deadline :-(. But dealing with a big patch logjam is obviously overwhelming the community's resources. I'm confident we can address that over time. It's easy for companies like NTT and EDB to start flooding us with big patches, but it takes time for us to start to trust their developers enough that 'regular' reviewers/committers can rely on them. We're already getting extremely high quality reviews from people like Heikki and over coming cycles I hope we'll get to trust more of the new flock of contributors who in turn will help relieve some of the workload. Regards, Dave ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] Memory leak in vac_update_relstats ?
Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid), 0, 0, 0); This copy is not subsequently freed in the function. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Memory leak in vac_update_relstats ?
Pavan Deolasee wrote: Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid), 0, 0, 0); This copy is not subsequently freed in the function. It's palloc'd in the current memory context, so it's not serious. It'll be freed at the end of the transaction, if not before that. That's the beauty of memory contexts; no need to worry about small allocations like that. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Memory leak in vac_update_relstats ?
Hi, It's palloc'd in the current memory context, so it's not serious. It'll be freed at the end of the transaction, if not before that. That's the beauty of memory contexts; no need to worry about small allocations like that. That's the beauty of memory contexts for small allocations. But because of the 'convenience' of memory contexts we sometimes tend to not pay attention to doing explicit pfrees. As a general rule I think allocations in TopMemoryContext should be critically examined. I was bitten by this undue bloat recently while developing some code and valgrind is not of much help in such cases because of this very beauty of memory contexts :). Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Memory leak in vac_update_relstats ?
Hi, That's the beauty of memory contexts for small allocations. But because of the 'convenience' of memory contexts we sometimes tend to not pay attention to doing explicit pfrees. As a general rule I think allocations in TopMemoryContext should be critically examined. I was bitten by this undue bloat recently while developing some code and valgrind is not of much help in such cases because of this very beauty of memory contexts :). One specific case I want to mention here is hash_create(). For local hash tables if HASH_CONTEXT is not specified, they get created in a context which becomes a direct child of TopMemoryContext. Wouldn't it be a better idea to create the table in CurrentMemoryContext? If hash_destroy() is not explicitly invoked, this can cause a lot of bloat especially if the intention was to use the hash table only for a while. Regards, Nikhils Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
[HACKERS] MAXIMUM_ALIGNOF on Windows-32
The MAXIMUM_ALIGNOF value is set to 8 bytes in a Windows- 32-bit environment. I have very little knowledge about Windows, but at the face of it, this looks strange. Any idea why is this required ? May be I am missing something obvious. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Memory leak in vac_update_relstats ?
NikhilS [EMAIL PROTECTED] writes: One specific case I want to mention here is hash_create(). For local hash tables if HASH_CONTEXT is not specified, they get created in a context which becomes a direct child of TopMemoryContext. Wouldn't it be a better idea to create the table in CurrentMemoryContext? It works that way partly for historical reasons and mostly because the vast majority of dynahash uses are for long-lived hash tables (a quick search says that a default of CurrentMemoryContext would be correct for only about one in ten of the current callers). I don't see any value in changing it. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] MAXIMUM_ALIGNOF on Windows-32
Pavan Deolasee [EMAIL PROTECTED] writes: The MAXIMUM_ALIGNOF value is set to 8 bytes in a Windows- 32-bit environment. I have very little knowledge about Windows, but at the face of it, this looks strange. Any idea why is this required ? It's not entirely unreasonable. The same thing happens on HPPA, which is nominally a 32-bit architecture but the hardware requires 8-byte alignment of doubles (and maybe int64 too, I forget). On newer Intel hardware it'd make sense to pad to avoid misaligned fetches. Anyway, we detect this directly based on the C compiler's behavior, and you can't argue with the compiler about it. Whatever it's doing is right by definition. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] MAXIMUM_ALIGNOF on Windows-32
Tom Lane wrote: Pavan Deolasee [EMAIL PROTECTED] writes: The MAXIMUM_ALIGNOF value is set to 8 bytes in a Windows- 32-bit environment. I have very little knowledge about Windows, but at the face of it, this looks strange. Any idea why is this required ? It's not entirely unreasonable. The same thing happens on HPPA, which is nominally a 32-bit architecture but the hardware requires 8-byte alignment of doubles (and maybe int64 too, I forget). On newer Intel hardware it'd make sense to pad to avoid misaligned fetches. Anyway, we detect this directly based on the C compiler's behavior, and you can't argue with the compiler about it. Whatever it's doing is right by definition. Perhaps Pavan is referring to what is hardcoded in pg_config.h.win32 which is used for MSVC builds (but not for MinGW builds, IIRC), in which case the answer might be that in this file we need to be pessimistic about such things, since we have no reasonable way to run configure on this platform. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] MAXIMUM_ALIGNOF on Windows-32
Andrew Dunstan [EMAIL PROTECTED] writes: Tom Lane wrote: Anyway, we detect this directly based on the C compiler's behavior, and you can't argue with the compiler about it. Whatever it's doing is right by definition. Perhaps Pavan is referring to what is hardcoded in pg_config.h.win32 which is used for MSVC builds (but not for MinGW builds, IIRC), in which case the answer might be that in this file we need to be pessimistic about such things, since we have no reasonable way to run configure on this platform. Somebody had better double-check that. We don't need to be pessimistic, we need to be *correct*, because the align values had better match the way the compiler will lay out a C struct. Otherwise struct-based access to catalog rows will fail. (I'm not sure if there are any system catalogs with float8 or int64 columns, but I'd sure not want to find out that we couldn't have one because of misconfiguration of MSVC builds.) I see though that the comment in pg_config.h.win32 claims it was derived from mechanically-generated configure output, so unless that's lying it should be OK already. AFAIK struct alignment is part of the ABI for a platform and is not subject to the whims of individual compilers, so the result from MinGW should be OK for MSVC. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] MAXIMUM_ALIGNOF on Windows-32
Tom Lane wrote: I see though that the comment in pg_config.h.win32 claims it was derived from mechanically-generated configure output, so unless that's lying it should be OK already. AFAIK struct alignment is part of the ABI for a platform and is not subject to the whims of individual compilers, so the result from MinGW should be OK for MSVC. Ah. OK, makes sense. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] MAXIMUM_ALIGNOF on Windows-32
On 7/20/07, Tom Lane [EMAIL PROTECTED] wrote: Pavan Deolasee [EMAIL PROTECTED] writes: The MAXIMUM_ALIGNOF value is set to 8 bytes in a Windows- 32-bit environment. I have very little knowledge about Windows, but at the face of it, this looks strange. Any idea why is this required ? It's not entirely unreasonable. The same thing happens on HPPA, which is nominally a 32-bit architecture but the hardware requires 8-byte alignment of doubles (and maybe int64 too, I forget). On newer Intel hardware it'd make sense to pad to avoid misaligned fetches. Anyway, we detect this directly based on the C compiler's behavior, and you can't argue with the compiler about it. Whatever it's doing is right by definition. Ah, that makes sense. I was confusing myself with 64-bit architectures and alignments. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Memory leak in vac_update_relstats ?
On 7/20/07, Heikki Linnakangas [EMAIL PROTECTED] wrote: Pavan Deolasee wrote: Are we leaking memory in vac_update_relstats ? /* Fetch a copy of the tuple to scribble on */ ctup = SearchSysCacheCopy(RELOID, ObjectIdGetDatum(relid), 0, 0, 0); This copy is not subsequently freed in the function. It's palloc'd in the current memory context, so it's not serious. It'll be freed at the end of the transaction, if not before that. That's the beauty of memory contexts; no need to worry about small allocations like that. Right. But may be for code completeness, we should add that missing heap_freetuple. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Memory leak in vac_update_relstats ?
Pavan Deolasee [EMAIL PROTECTED] writes: On 7/20/07, Heikki Linnakangas [EMAIL PROTECTED] wrote: It's palloc'd in the current memory context, so it's not serious. Right. But may be for code completeness, we should add that missing heap_freetuple. Personally I've been thinking of mounting an effort to get rid of unnecessary pfree's wherever possible. Particularly in user-defined functions, cleaning up at the end is a waste of code space and cycles too, because they're typically called in contexts that are going to be reset immediately afterward. In the case of vac_update_relstats, it's called only once per transaction, so there's certainly no point in being a neatnik. Stuff you need to worry about is functions that might be called many times in the same memory context. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Memory leak in vac_update_relstats ?
Tom Lane [EMAIL PROTECTED] writes: Personally I've been thinking of mounting an effort to get rid of unnecessary pfree's wherever possible. Particularly in user-defined functions, cleaning up at the end is a waste of code space and cycles too, because they're typically called in contexts that are going to be reset immediately afterward. It seems like the impact of this is self-limiting though. The worst-case is going to be something which executes an extra pfree for every tuple. Or perhaps one for every expression in a complex query involving lots of expressions. Saving a few extra pfrees per tuple isn't really going to buy many cpu cycles. In the case of vac_update_relstats, it's called only once per transaction, so there's certainly no point in being a neatnik. Stuff you need to worry about is functions that might be called many times in the same memory context. Fwiw there are user-space functions that can't leak memory. I'm sure everyone knows about btree operators, but pgsql also assumes that type input and output functions don't leak memory too (or else assignment leaks memory in the function scope memory context and in a loop...). -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [GENERAL] 8.2.4 signal 11 with large transaction
Bill Moran [EMAIL PROTECTED] writes: Oddly, the query succeeds if it's fed into psql. I'm now full of mystery and wonder. It would appear as if the underlying problem has something to do with PHP, but why should this cause a backend process to crash? Ah, I see it. Your PHP script is sending all 3 INSERT commands to the backend *in a single PQexec*, ie, one 37MB command string. psql won't do that, it splits the input at semicolons. Unsurprisingly, this runs the backend out of memory. (It's not the command string that's the problem, so much as the 3 parse and plan trees...) Unfortunately, in trying to prepare the error message, it tries to attach the command text as the STATEMENT field of the log message. All 37MB worth. And of course *that* gets an out-of-memory error. Presto, infinite recursion, broken only by stack overflow (= SIGSEGV). It looks like 8.1 and older are also vulnerable to this, it's just that they don't try to log error statement strings at the default logging level, whereas 8.2 does. If you cranked up log_min_error_statement I think they'd fail too. I guess what we need to do is hack the emergency-recovery path for error-during-error-processing such that it will prevent trying to print a very long debug_query_string. Maybe we should just not try to print the command at all in this case, or maybe there's some intermediate possibility like only printing the first 1K or so. Thoughts? regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Memory leak in vac_update_relstats ?
Gregory Stark [EMAIL PROTECTED] writes: It seems like the impact of this is self-limiting though. The worst-case is going to be something which executes an extra pfree for every tuple. Or perhaps one for every expression in a complex query involving lots of expressions. Saving a few extra pfrees per tuple isn't really going to buy many cpu cycles. I can't tell you how many profiles I've looked at in which palloc/pfree were *the* dominant consumers of CPU cycles. I'm not sure how much could be saved this particular way, but I wouldn't dismiss it as uninteresting. I've actually thought about making short-term memory contexts use a variant MemoryContext type in which pfree was a no-op and palloc was simplified by not worrying at all about recycling space. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Memory leak in vac_update_relstats ?
Tom Lane wrote: I've actually thought about making short-term memory contexts use a variant MemoryContext type in which pfree was a no-op and palloc was simplified by not worrying at all about recycling space. That sounds like a good idea to me. cheers andrew ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
