Re: [PATCHES] enable logging of start time/cookie for all backend processes
Alvaro Herrera wrote: Andrew Dunstan wrote: The attached patch makes a very small but useful change to the behaviour of log_line_prefix, by enabling the start time (%s) and cookie (%c) logging to occur for all backends rather than just for session processes (i.e. backends started for a client connection). We actually need almost all of this patch, with or without the change in behaviour, so we can put the cookie in CSVlogs (which I'm still working on), since the cookie+line number make the natural primary key for the logs. The actual change in behaviour from this patch comes from the removal of 2 "if (MyProcPort)" lines in elog.c. Given that, can I sneak this in or should I wait for 8.4 given we're long past feature freeze? Thinking again about the feature itself I wonder if it actually makes sense -- maybe it does make sense to be able to display the session ID, but the start time? Why would anyone care about the start time of syslogger or bgwriter? We don't even have a use for the "hey, this process was started" log line, why would anyone care about having the start time in the log line prefix? Actually having the cookie in all processes is another matter, as far as it's useful for CSV logs. But then, is it? Maybe the auxiliary processes should identify themselves with fixed cookies or something particular that lets one distinguish, say, a bgwriter from a syslogger, but is there a case from distinguishing one bgwriter from another? It's not about distinguishing one bgwriter from another, it's about distinguishing it from any other process at any time whatsoever that has had the same pid. cookie+linenumber should be unique. pid+linenumber isn't. (And every process gets its own line number sequence, so we can't just give, say, all the bgwriter processes the same cookie). Logging the start time on its own isn't much extra benefit, although I expect log parsers will find it nicer to be able to handle a more consistent logging style rather than having to handle non-session processes as a special case. But having the cookie available in all cases is the whole point of this - I wouldn't have done it unless I had needed to be able to set a primary key for loadable logs. If you want to invent some other style of cookie we can look at that. Back when we looked at it originally nobody came up with a better suggestion than process_start.pid. But that surely would be for a later release ;-) So, short answer - yes, I think it makes sense. But if there's any serious argument I won't change the observable behaviour in elog.c, just the infrastructure. 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
Re: [PATCHES] HOT patch - version 11
On 8/2/07, Pavan Deolasee <[EMAIL PROTECTED]> wrote: > > > > On 8/2/07, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: > > > > Maybe a nicer > > solution would be to have another version of ConditionalLockBuffer with > > three different return values: didn't get lock, got exclusive lock, or > > got cleanup lock. > > > > Thats a good idea. I shall do that. > > On a second thought, I feel its may not be such a good idea to change the ConditionalLockBuffer return value. "boolean" is the most natural way. And we don't save anything in terms on BufHdr locking. So may be should just have two different functions and do the BufferIsLockedForCleanup check immediately after acquiring the exclusive lock. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] HOT patch - version 11
On 8/2/07, Heikki Linnakangas <[EMAIL PROTECTED]> wrote: > > Pavan Deolasee wrote: > > Please see the attached version 11 of HOT patch > > Thanks! > > One wrinkle in the patch is how the ResultRelInfo-struct is passed to > heap_update, and on to heap_check_idxupdate, to check any indexed > columns have changed. I think that's a modularity violation, heap_update > really shouldn't have to deal with ResultRelInfo, that belongs in the > executor. When we add support for expression and partial indexes, > heap_check_idxupdate will need even more executor machinery to be able > to evaluate expressions. The reason I put it there because we wanted to do that check as late as possible, once we confirm that update is possible and there is enough space in the block to perform HOT update. But I agree thats a modularity violation. Any suggestion to avoid that ? In heap_page_prune_defrag, it would be better to do the test for > BufferIsLockedForCleanup right after acquiring the lock. The longer the > delay between those steps, the bigger the chances that someone pins the > page and starts to wait for the buffer lock, making us think that we > didn't get the cleanup lock, though we actually did. Maybe a nicer > solution would be to have another version of ConditionalLockBuffer with > three different return values: didn't get lock, got exclusive lock, or > got cleanup lock. Thats a good idea. I shall do that. It's not necessary to WAL-log the unused-array that > PageRepairFragmentation returns. In replay, a call to > PageRepairFragmentation will come to the same conclusion about which > line pointers are not used. It would also be better if we didn't emit a > separate WAL record for defraging a page, if we also prune it at the > same time. I'm not that worried about WAL usage in general, but that > seems simple enough to fix. Ah I see. I shall fix that. Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] HOT patch - version 11
On Wed, 2007-08-01 at 21:09 +0100, Heikki Linnakangas wrote: > In heap_page_prune_defrag, it would be better to do the test for > BufferIsLockedForCleanup right after acquiring the lock. The longer the > delay between those steps, the bigger the chances that someone pins the > page and starts to wait for the buffer lock, making us think that we > didn't get the cleanup lock, though we actually did. Maybe a nicer > solution would be to have another version of ConditionalLockBuffer with > three different return values: didn't get lock, got exclusive lock, or > got cleanup lock. Yeh, 3-value return seems neatest way. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(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: [PATCHES] Memory leak in tuplestore_end()
On Wed, 2007-08-01 at 16:49 -0700, Neil Conway wrote: > Attached is a patch which fixes a memory leak in tuplestore_end(). Applied to HEAD, and to back branches as far back as 7.4. -Neil ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] use binary mode on syslog pipe on windows to avoid upsetting chunking protocol
Andrew Dunstan wrote: This small patch makes the syslog pipe use binary mode on Windows This is now committed and ported back to 8.0. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] enable logging of start time/cookie for all backend processes
Andrew Dunstan wrote: Alvaro Herrera wrote: Andrew Dunstan wrote: The attached patch makes a very small but useful change to the behaviour of log_line_prefix, by enabling the start time (%s) and cookie (%c) logging to occur for all backends rather than just for session processes (i.e. backends started for a client connection). We actually need almost all of this patch, with or without the change in behaviour, so we can put the cookie in CSVlogs (which I'm still working on), since the cookie+line number make the natural primary key for the logs. The actual change in behaviour from this patch comes from the removal of 2 "if (MyProcPort)" lines in elog.c. Given that, can I sneak this in or should I wait for 8.4 given we're long past feature freeze? Thinking again about the feature itself I wonder if it actually makes sense -- maybe it does make sense to be able to display the session ID, but the start time? Why would anyone care about the start time of syslogger or bgwriter? We don't even have a use for the "hey, this process was started" log line, why would anyone care about having the start time in the log line prefix? Actually having the cookie in all processes is another matter, as far as it's useful for CSV logs. But then, is it? Maybe the auxiliary processes should identify themselves with fixed cookies or something particular that lets one distinguish, say, a bgwriter from a syslogger, but is there a case from distinguishing one bgwriter from another? It's not about distinguishing one bgwriter from another, it's about distinguishing it from any other process at any time whatsoever that has had the same pid. cookie+linenumber should be unique. pid+linenumber isn't. (And every process gets its own line number sequence, so we can't just give, say, all the bgwriter processes the same cookie). Logging the start time on its own isn't much extra benefit, although I expect log parsers will find it nicer to be able to handle a more consistent logging style rather than having to handle non-session processes as a special case. But having the cookie available in all cases is the whole point of this - I wouldn't have done it unless I had needed to be able to set a primary key for loadable logs. If you want to invent some other style of cookie we can look at that. Back when we looked at it originally nobody came up with a better suggestion than process_start.pid. But that surely would be for a later release ;-) So, short answer - yes, I think it makes sense. But if there's any serious argument I won't change the observable behaviour in elog.c, just the infrastructure. In the absence of further discussion I have committed this. That clears the decks for me to have yet another go at CSVlogs ;-) cheers andrew ---(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
