Thread: log_filename_prefix --> log_filename + strftime()
Attached is a patch which replaces the 'log_filename_prefix' configuration
directive with a similar 'log_filename' directive. It differs from the
former in the following ways:
+ allows embedded strftime() escapes ala Apache's rotatelogs;
+ eliminates hard-coded embedding of the postmaster pid;
+ makes the current hard-coded timestamp configurable;
+ changes the default log filename to exclude the PID;
This patch enables us to continue using our existing log-handling utilities
and filenaming conventions which we now use with Apache's rotatelogs.
Attachment
The patch is intended for 8.0.0 or later, and was generated and tested with the cvs trunk as of 26-Aug-2004. On Friday August 27 2004 11:50, Ed L. wrote: > Attached is a patch which replaces the 'log_filename_prefix' > configuration directive with a similar 'log_filename' directive. It > differs from the former in the following ways: > > + allows embedded strftime() escapes ala Apache's rotatelogs; > + eliminates hard-coded embedding of the postmaster pid; > + makes the current hard-coded timestamp configurable; > + changes the default log filename to exclude the PID; > > This patch enables us to continue using our existing log-handling > utilities and filenaming conventions which we now use with Apache's > rotatelogs.
"Ed L." <pgsql@bluepolka.net> writes:
> Attached is a patch which replaces the 'log_filename_prefix' configuration
> directive with a similar 'log_filename' directive.
> + changes the default log filename to exclude the PID;
This would be better stated as "makes it impossible to use the PID
in the file name". While I'm prepared to grant that it may not be
necessary to do so in many scenarios, I'm not very happy with
arbitrarily removing the ability ... especially without giving any
justification.
regards, tom lane
On Friday August 27 2004 12:08, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > > Attached is a patch which replaces the 'log_filename_prefix' > > configuration directive with a similar 'log_filename' directive. > > + changes the default log filename to exclude the PID; > > This would be better stated as "makes it impossible to use the PID > in the file name". While I'm prepared to grant that it may not be > necessary to do so in many scenarios, I'm not very happy with > arbitrarily removing the ability ... especially without giving any > justification. Yes, should have said more on that item. First, I didn't see how to easily make it configurable in combination with strftime() without doing more work, and it didn't appear to be worth the effort. By its addition, hard-coding the PID into the filename deviates from what I would argue is the de facto standard of Apache's rotatelogs and forces a naming convention where none existed before. That creates work for us as we have a considerable infrastructure setup to deal with logs; I suspect that may be the case with others. I looked, but did not find, justification for why it was introduced; I would assume it was added to allow for multiple postmasters sharing the same log directory. I had difficulty fathoming the usefulness of this being hard-coded, as it seems one could compensate easily through the configurable 'log_filename' if one chose to share a log directory among postmasters. Not by including the PID, but by some other postmaster-unique naming approach. Given its a new 'feature', I'm hoping it can be altered to return the freedom of filenaming to the administrator.
"Ed L." <pgsql@bluepolka.net> writes:
> On Friday August 27 2004 12:08, Tom Lane wrote:
>> [ justification please ]
> Yes, should have said more on that item. First, I didn't see how to easily
> make it configurable in combination with strftime() without doing more
> work, and it didn't appear to be worth the effort. By its addition,
> hard-coding the PID into the filename deviates from what I would argue is
> the de facto standard of Apache's rotatelogs and forces a naming convention
> where none existed before. That creates work for us as we have a
> considerable infrastructure setup to deal with logs; I suspect that may be
> the case with others. I looked, but did not find, justification for why it
> was introduced; I would assume it was added to allow for multiple
> postmasters sharing the same log directory. I had difficulty fathoming the
> usefulness of this being hard-coded, as it seems one could compensate
> easily through the configurable 'log_filename' if one chose to share a log
> directory among postmasters. Not by including the PID, but by some other
> postmaster-unique naming approach. Given its a new 'feature', I'm hoping
> it can be altered to return the freedom of filenaming to the administrator.
Or you could use different log_directory settings for different PMs.
Fair enough.
Anyone else have an opinion pro or con about this change? IMHO it's in
the gray area between bug fix and feature addition. If we want to do
it, though, doing it now is certainly better than holding it for 8.1,
since by then people would have gotten used to the present behavior.
BTW, as long as we are taking Apache as the de facto standard --- does
the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or
would something different be closer to the common practice with Apache?
regards, tom lane
On Friday August 27 2004 12:41, Tom Lane wrote: > > BTW, as long as we are taking Apache as the de facto standard --- does > the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or > would something different be closer to the common practice with Apache? Apache defaults to access_log.N where N is the epoch of the logfile start time. Ed
On Friday August 27 2004 12:51, Ed L. wrote: > On Friday August 27 2004 12:41, Tom Lane wrote: > > BTW, as long as we are taking Apache as the de facto standard --- does > > the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or > > would something different be closer to the common practice with Apache? > > Apache defaults to access_log.N where N is the epoch of the logfile start > time. I should say, Apache rotatelogs takes a configurable filename and then appends ".N" where N is the logfile start time epoch. In one case, its access_log.N, in another its error_log.N. Ed
Tom Lane wrote: >"Ed L." <pgsql@bluepolka.net> writes: > > >>On Friday August 27 2004 12:08, Tom Lane wrote: >> >> >>>[ justification please ] >>> >>> > > > >>Yes, should have said more on that item. First, I didn't see how to easily >>make it configurable in combination with strftime() without doing more >>work, and it didn't appear to be worth the effort. By its addition, >>hard-coding the PID into the filename deviates from what I would argue is >>the de facto standard of Apache's rotatelogs and forces a naming convention >>where none existed before. That creates work for us as we have a >>considerable infrastructure setup to deal with logs; I suspect that may be >>the case with others. I looked, but did not find, justification for why it >>was introduced; I would assume it was added to allow for multiple >>postmasters sharing the same log directory. I had difficulty fathoming the >>usefulness of this being hard-coded, as it seems one could compensate >>easily through the configurable 'log_filename' if one chose to share a log >>directory among postmasters. Not by including the PID, but by some other >>postmaster-unique naming approach. Given its a new 'feature', I'm hoping >>it can be altered to return the freedom of filenaming to the administrator. >> >> > >Or you could use different log_directory settings for different PMs. >Fair enough. > >Anyone else have an opinion pro or con about this change? IMHO it's in >the gray area between bug fix and feature addition. If we want to do >it, though, doing it now is certainly better than holding it for 8.1, >since by then people would have gotten used to the present behavior. > >BTW, as long as we are taking Apache as the de facto standard --- does >the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or >would something different be closer to the common practice with Apache? > > > > If the PID isn't there is there a danger of different postmasters clobbering each other's logs? ISTM having the PID there gives some sort of guarantee that that won't happen. I don't have any strong opinion one way or another, but I wondered if a configurable strings with % escapes like we use for log_line_prefix might be better. It could be argued to be overkill, I guess. Alternatively, we could have a different boolean option 'log_filename_use_pid' or some such. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> If the PID isn't there is there a danger of different postmasters
> clobbering each other's logs?
Only if they're logging into the same directory with the same filename
pattern. This isn't the default (the default log_directory is under
$PGDATA), and so I'm not that worried now that I think about it carefully.
I'm coming around to agree with Ed's position that it's not worth the
trouble to provide more than the strftime escapes.
regards, tom lane
"Ed L." <pgsql@bluepolka.net> writes:
> On Friday August 27 2004 12:41, Tom Lane wrote:
>> BTW, as long as we are taking Apache as the de facto standard --- does
>> the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or
>> would something different be closer to the common practice with Apache?
> I should say, Apache rotatelogs takes a configurable filename and then
> appends ".N" where N is the logfile start time epoch. In one case, its
> access_log.N, in another its error_log.N.
Hmm ... there isn't any way to emulate that with strftime escapes,
unless I missed the right one.
regards, tom lane
On Friday August 27 2004 1:03, Tom Lane wrote:
> "Ed L." <pgsql@bluepolka.net> writes:
> > On Friday August 27 2004 12:41, Tom Lane wrote:
> >> BTW, as long as we are taking Apache as the de facto standard --- does
> >> the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense,
> >> or would something different be closer to the common practice with
> >> Apache?
> >
> > I should say, Apache rotatelogs takes a configurable filename and then
> > appends ".N" where N is the logfile start time epoch. In one case, its
> > access_log.N, in another its error_log.N.
>
> Hmm ... there isn't any way to emulate that with strftime escapes,
> unless I missed the right one.
If you supply an escape, Apache will override that default epoch. So I
could see setting the default to "server_log" or "postgresql_log" or
whatever, and making the default (with no escapes supplied) be the epoch.
That would be easy tweak, and be much closer to Apache style.
Ed
Apache 1.3.31:
if (use_strftime) {
struct tm *tm_now;
tm_now = gmtime(&tLogStart);
strftime(buf2, sizeof(buf2), szLogRoot, tm_now);
}
else {
sprintf(buf2, "%s.%010d", szLogRoot, (int) tLogStart);
}
Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > >>Attached is a patch which replaces the 'log_filename_prefix' configuration >>directive with a similar 'log_filename' directive. >> + changes the default log filename to exclude the PID; > > > This would be better stated as "makes it impossible to use the PID > in the file name". While I'm prepared to grant that it may not be > necessary to do so in many scenarios, I'm not very happy with > arbitrarily removing the ability ... especially without giving any > justification. I don't have the time now to review the impact, but this might make interpreting the log filename difficult or impossible, effectively corrupting pg_logdir_ls. I don't object against adjusting the timestamp format in a reasonable way, but it should stay fixed; same about PID. Regards, Andreas
"Ed L." <pgsql@bluepolka.net> writes:
> On Friday August 27 2004 1:03, Tom Lane wrote:
>> Hmm ... there isn't any way to emulate that with strftime escapes,
>> unless I missed the right one.
> If you supply an escape, Apache will override that default epoch. So I
> could see setting the default to "server_log" or "postgresql_log" or
> whatever, and making the default (with no escapes supplied) be the epoch.
> That would be easy tweak, and be much closer to Apache style.
Yeah, and it would also prevent a risk I now see with your initial
patch: if no %, it'll write the same filename each time, which
is almost certainly not desired. Works for me.
regards, tom lane
Tom Lane wrote: >"Ed L." <pgsql@bluepolka.net> writes: > > >>On Friday August 27 2004 12:41, Tom Lane wrote: >> >> >>>BTW, as long as we are taking Apache as the de facto standard --- does >>>the default of "postgresql-%Y-%m-%d_%H%M%S.log" actually make sense, or >>>would something different be closer to the common practice with Apache? >>> >>> > > > >>I should say, Apache rotatelogs takes a configurable filename and then >>appends ".N" where N is the logfile start time epoch. In one case, its >>access_log.N, in another its error_log.N. >> >> > >Hmm ... there isn't any way to emulate that with strftime escapes, >unless I missed the right one. > > > > According to my Linux man page, the Olsen library has %s for that. I don't see it in src/timezone/strftime.c, though cheers andrew
Andreas Pflug <pgadmin@pse-consulting.de> writes:
> I don't have the time now to review the impact, but this might make
> interpreting the log filename difficult or impossible, effectively
> corrupting pg_logdir_ls.
So if you want to use that, you use a format that it can cope with.
I don't see a problem.
(This is probably an OK reason to keep the default log_filename in
Y/M/D/H/M/S style, though, rather than ".epoch" style.)
regards, tom lane
Tom Lane wrote: > Andreas Pflug <pgadmin@pse-consulting.de> writes: > > I don't have the time now to review the impact, but this might make > > interpreting the log filename difficult or impossible, effectively > > corrupting pg_logdir_ls. > > So if you want to use that, you use a format that it can cope with. > I don't see a problem. > > (This is probably an OK reason to keep the default log_filename in > Y/M/D/H/M/S style, though, rather than ".epoch" style.) Ah, so we keep the existing format but drop the pid, and just make it changable by the user, and we rename it. Doesn't sound as drastic as it first did. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Ah, so we keep the existing format but drop the pid, and just make it
> changable by the user, and we rename it. Doesn't sound as drastic as it
> first did.
Yeah, the only change in default behavior would be to drop the PID part
of the log filename, which doesn't seem too bad, since people aren't yet
depending on that.
regards, tom lane
On Friday August 27 2004 1:39, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Ah, so we keep the existing format but drop the pid, and just make it
> > changable by the user, and we rename it. Doesn't sound as drastic as
> > it first did.
>
> Yeah, the only change in default behavior would be to drop the PID part
> of the log filename, which doesn't seem too bad, since people aren't yet
> depending on that.
>
> regards, tom lane
OK, if I read you correctly...
Default remains "postgresql-%Y-%m-%d_%H%M%S.log"
(Apache style: access_log.%s)
If log_filename = 'xxx', rotate with strftime() to 'xxx-%Y-%m-%d_%H%M%S'
(Apache style: xxx.%s)
If log_filename = 'xxx.%a', rotate with strftime() to 'xxx.%a'
(Apache style: xxx.%a)
Not a big fan of the verbose 32-character default name, 'server_log.%s'
would be my pick, but easy enough to override it.
Ed
"Ed L." <pgsql@bluepolka.net> writes:
> If log_filename = 'xxx', rotate with strftime() to 'xxx-%Y-%m-%d_%H%M%S'
No, I was thinking that if no %'s in the log_filename, then use xxx.EPOCH
to provide Apache compatibility.
regards, tom lane
On Friday August 27 2004 2:15, Tom Lane wrote:
> "Ed L." <pgsql@bluepolka.net> writes:
> > If log_filename = 'xxx', rotate with strftime() to
> > 'xxx-%Y-%m-%d_%H%M%S'
>
> No, I was thinking that if no %'s in the log_filename, then use xxx.EPOCH
> to provide Apache compatibility.
OK, that works for me.
One addition I'd like to include with the revised patch: a boolean
postgresql.conf option ('log_truncate_on_rotation', default false) to
truncate any existing log file by the same name. Default behavior here and
with Apache is to always append, but it's a useful feature for us because
it largely eliminates the issue of logs filling up the disk. You don't
want the log clobbered on restarts, so the idea is to only truncate during
time/size-based rotation, not on the initial open. Thoughts?
Ed
On Friday August 27 2004 1:15, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > > On Friday August 27 2004 1:03, Tom Lane wrote: > >> Hmm ... there isn't any way to emulate that with strftime escapes, > >> unless I missed the right one. > > > > If you supply an escape, Apache will override that default epoch. So I > > could see setting the default to "server_log" or "postgresql_log" or > > whatever, and making the default (with no escapes supplied) be the > > epoch. That would be easy tweak, and be much closer to Apache style. > > Yeah, and it would also prevent a risk I now see with your initial > patch: if no %, it'll write the same filename each time, which > is almost certainly not desired. Works for me. I think this turns out to be no big deal either way here as it is for Apache either way. Consider if I set my rotation time to 1 hour and my log_filename = 'server_log.%a' (server_log.Fri). Then each of the first 22 rotations for the day will simply reopen and append to the same file. IIRC, Apache's rotatelogs works the same way. In both cases, you just have to be careful to coordinate your filename and rotation time/size limits to get the desired effect. Ed
"Ed L." <pgsql@bluepolka.net> writes:
> One addition I'd like to include with the revised patch: a boolean
> postgresql.conf option ('log_truncate_on_rotation', default false) to
> truncate any existing log file by the same name. Default behavior here and
> with Apache is to always append, but it's a useful feature for us because
> it largely eliminates the issue of logs filling up the disk. You don't
> want the log clobbered on restarts, so the idea is to only truncate during
> time/size-based rotation, not on the initial open. Thoughts?
Hmm, so if you use e.g. "postgresql_%H.log", you always have 24 logfiles
of one hour each, and you don't need any explicit code at all to remove
old logs. Not a bad idea. It's definitely creeping featurism ... but
I can see the value of not needing any cron daemon to remove old logs.
A potential problem is what about size-driven rotation? If the hourly
output exceeds log_rotation_size then you'd truncate and rewrite the
current file, which is just exactly not what you want :-(. You could
say that truncation occurs only at time-driven, not size-driven
rotations, but that would effectively amount to saying that size-driven
rotation is disabled, which I don't think I like ...
One other thing I've been thinking of suggesting is that the
next-rotation-target-time be rounded to an exact multiple of
log_rotation_age. So for example if you set log_rotation_age = 60
minutes then rotations will happen at the top of the hour no matter
when the postmaster was started. The simplistic approach of doing
this on the time_t value would mean that, say, age = 24*60 would give
you rotations occurring at GMT midnight not local midnight, which isn't
perfect but I'd say good enough. Again though, the interaction with
size-driven rotation might need more thought.
Possibly you could fix the first issue if you did all this to the code
and then used, say, log_filename "postgresql_%H:%M.log" with 60-minute
rotation. You'd normally get only logfiles named after the top of the
hour, but in an hour with unusually heavy log output you might get some
additional files with intermediate %M values. Course that puts you back
to needing a cron daemon to clean those up ...
regards, tom lane
"Ed L." <pgsql@bluepolka.net> writes:
> On Friday August 27 2004 1:15, Tom Lane wrote:
>> Yeah, and it would also prevent a risk I now see with your initial
>> patch: if no %, it'll write the same filename each time, which
>> is almost certainly not desired. Works for me.
> I think this turns out to be no big deal either way here as it is for Apache
> either way. Consider if I set my rotation time to 1 hour and my
> log_filename = 'server_log.%a' (server_log.Fri).
But that avoids the problem because you *do* have an escape, and thus
more than one possible logfilename. If we treat no-escape as meaning
a constant filename, there is no rotation possible, other than rotation
through truncation which doesn't seem likely to be useful to anyone.
regards, tom lane
On Friday August 27 2004 3:49, Tom Lane wrote: > > A potential problem is what about size-driven rotation? If the hourly > output exceeds log_rotation_size then you'd truncate and rewrite the > current file, which is just exactly not what you want :-(. You could > say that truncation occurs only at time-driven, not size-driven > rotations, but that would effectively amount to saying that size-driven > rotation is disabled, which I don't think I like ... > One other thing I've been thinking of suggesting is that the > next-rotation-target-time be rounded to an exact multiple of > log_rotation_age. So for example if you set log_rotation_age = 60 > minutes then rotations will happen at the top of the hour no matter > when the postmaster was started. The simplistic approach of doing > this on the time_t value would mean that, say, age = 24*60 would give > you rotations occurring at GMT midnight not local midnight, which isn't > perfect but I'd say good enough. Again though, the interaction with > size-driven rotation might need more thought. Apache's rotatelogs works this way, and includes a UTC offset option, to allow rotations at local midnight. > Possibly you could fix the first issue if you did all this to the code > and then used, say, log_filename "postgresql_%H:%M.log" with 60-minute > rotation. You'd normally get only logfiles named after the top of the > hour, but in an hour with unusually heavy log output you might get some > additional files with intermediate %M values. Course that puts you back > to needing a cron daemon to clean those up ... Not that elegant, but pretty reasonable, I think. In the normal case of logfiles under the maximum size, everything is cleaned up. If you bloat, you have some clean-up to do, but easy enough with a cron job. We have been operating ~40 clusters this way for a couple years now with a modified Apache rotatelogs (w/truncate option) and a cron to clean-up too-old logfiles. It has pretty much eliminated our disk-full crises from DB logs. Ed
On Friday August 27 2004 4:34, Ed L. wrote: > > One other thing I've been thinking of suggesting is that the > > next-rotation-target-time be rounded to an exact multiple of > > log_rotation_age. So for example if you set log_rotation_age = 60 > > minutes then rotations will happen at the top of the hour no matter > > when the postmaster was started. The simplistic approach of doing > > this on the time_t value would mean that, say, age = 24*60 would give > > you rotations occurring at GMT midnight not local midnight, which isn't > > perfect but I'd say good enough. Again though, the interaction with > > size-driven rotation might need more thought. > > Apache's rotatelogs works this way, and includes a UTC offset option, to > allow rotations at local midnight. I see struct pg_tm has tm_gmtoff, but it seems to be zero on my MST7MDT 2.4 kernel linux box here. Is there a standard way of retrieving the offset within the PG source code? Ed
Tom Lane wrote: > It's definitely creeping featurism ... but > I can see the value of not needing any cron daemon to remove old logs. No other logs on your system to purge? > > A potential problem is what about size-driven rotation? If the hourly > output exceeds log_rotation_size then you'd truncate and rewrite the > current file, which is just exactly not what you want :-(. Same can happen after logger process restart. After all, I wonder what an apache style logfile name is good for. The pgsql logfiles don't contain access log data that are analyzed by webalizer or stuff like that. I don't see the point having any formatting option at all. Regards, Andreas
Andreas Pflug <pgadmin@pse-consulting.de> writes:
> Tom Lane wrote:
>> I can see the value of not needing any cron daemon to remove old logs.
> No other logs on your system to purge?
The DBA isn't necessarily also root.
regards, tom lane
Are we going to change this before beta2? I have not seen a final patch yet. --------------------------------------------------------------------------- Ed L. wrote: > Attached is a patch which replaces the 'log_filename_prefix' configuration > directive with a similar 'log_filename' directive. It differs from the > former in the following ways: > > + allows embedded strftime() escapes ala Apache's rotatelogs; > + eliminates hard-coded embedding of the postmaster pid; > + makes the current hard-coded timestamp configurable; > + changes the default log filename to exclude the PID; > > This patch enables us to continue using our existing log-handling utilities > and filenaming conventions which we now use with Apache's rotatelogs. > [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Andreas Pflug <pgadmin@pse-consulting.de> writes: > >>Tom Lane wrote: >> >>>I can see the value of not needing any cron daemon to remove old logs. > > >>No other logs on your system to purge? > > > The DBA isn't necessarily also root. Interesting this argument comes from you.. :-) Tasks like purging old log files is certainly not a job that needs to be implemented in the backend; instead, an external database maintenance agent should do that. Such an agent (pgadmin TODO list working title: pgAgent, there was a lengthy discussion "Scheduled jobs" starting 2004-05-12), allowing scheduled sql scripts, would delete old log files using select pg_file_unlink(filename) from pg_logdir_ls where filetime < now() - '8 days'::interval *if* this functionality isn't corrupted by arbitrary selectable file name formatting. Regards, Andreas
Bruce Momjian wrote: > Are we going to change this before beta2? I have not seen a final > patch yet. Can we have pg_logdir_ls in the backend first so any related changes to the log filename are reflected in both places? Otherwise displaying the logfile on the client continues to be a moving target. Regards, Andreas
On 8/27/2004 2:41 PM, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: >> On Friday August 27 2004 12:08, Tom Lane wrote: >>> [ justification please ] > >> Yes, should have said more on that item. First, I didn't see how to easily >> make it configurable in combination with strftime() without doing more >> work, and it didn't appear to be worth the effort. By its addition, >> hard-coding the PID into the filename deviates from what I would argue is >> the de facto standard of Apache's rotatelogs and forces a naming convention >> where none existed before. That creates work for us as we have a >> considerable infrastructure setup to deal with logs; I suspect that may be >> the case with others. I looked, but did not find, justification for why it >> was introduced; I would assume it was added to allow for multiple >> postmasters sharing the same log directory. I had difficulty fathoming the >> usefulness of this being hard-coded, as it seems one could compensate >> easily through the configurable 'log_filename' if one chose to share a log >> directory among postmasters. Not by including the PID, but by some other >> postmaster-unique naming approach. Given its a new 'feature', I'm hoping >> it can be altered to return the freedom of filenaming to the administrator. > > Or you could use different log_directory settings for different PMs. > Fair enough. > > Anyone else have an opinion pro or con about this change? IMHO it's in > the gray area between bug fix and feature addition. If we want to do > it, though, doing it now is certainly better than holding it for 8.1, > since by then people would have gotten used to the present behavior. Which is thw way we have treated things the like before. I remember that by the time lztext was in the backend we knew already about toast in the next release and therefore discouraged the use. This is not possible here, so I say do the change now. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
On 8/29/2004 5:12 AM, Andreas Pflug wrote: > Tom Lane wrote: >> Andreas Pflug <pgadmin@pse-consulting.de> writes: >> >>>Tom Lane wrote: >>> >>>>I can see the value of not needing any cron daemon to remove old logs. >> >> >>>No other logs on your system to purge? >> >> >> The DBA isn't necessarily also root. > > Interesting this argument comes from you.. :-) > > Tasks like purging old log files is certainly not a job that needs to be > implemented in the backend; instead, an external database maintenance > agent should do that. You must have misunderstood something here. The proposal doesn't implement any logfile purging feature, but allows to setup a configuration that automatically overwrites files in a rotating manner, if the DBA so desires. I can't see how you're jumping to logfile purging from that. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck wrote: but allows to setup a > configuration that automatically overwrites files in a rotating manner, > if the DBA so desires. ... which can't work because it will overwrite the logfile on server start, and thus will overwrite the very latest logfile when performing multiple restarts. We had discussions how to identify a logfile's start time, and agreed that the file's creation/modification time can *not* be used for that. That's why the name has a fixed and well-known timestamp format. Regards, Andreas
Andreas Pflug <pgadmin@pse-consulting.de> writes:
> Jan Wieck wrote:
>> but allows to setup a
>> configuration that automatically overwrites files in a rotating manner,
>> if the DBA so desires.
> ... which can't work because it will overwrite the logfile on server
> start, and thus will overwrite the very latest logfile when performing
> multiple restarts.
You are ignoring a critical part of the proposal, which is to overwrite
only during a time-based rotation; at logger startup or size-based
rotation, the rule would be to append.
regards, tom lane
On 8/29/2004 2:06 PM, Tom Lane wrote: > Andreas Pflug <pgadmin@pse-consulting.de> writes: >> Jan Wieck wrote: >>> but allows to setup a >>> configuration that automatically overwrites files in a rotating manner, >>> if the DBA so desires. > >> ... which can't work because it will overwrite the logfile on server >> start, and thus will overwrite the very latest logfile when performing >> multiple restarts. > > You are ignoring a critical part of the proposal, which is to overwrite > only during a time-based rotation; at logger startup or size-based > rotation, the rule would be to append. which then has a problem when you startup the postmaster after 10 hours of downtime ... hmmm. Jan -- #======================================================================# # It's easier to get forgiveness for being wrong than for being right. # # Let's break this rule - forgive me. # #================================================== JanWieck@Yahoo.com #
Jan Wieck <JanWieck@Yahoo.com> writes:
>> You are ignoring a critical part of the proposal, which is to overwrite
>> only during a time-based rotation; at logger startup or size-based
>> rotation, the rule would be to append.
> which then has a problem when you startup the postmaster after 10 hours
> of downtime ... hmmm.
Doesn't seem like a big problem --- at worst that logfile will get to be
double the size it normally would.
Note that this scheme effectively disables size-based rotation anyway,
unless you use one of the hacks we talked about like using a %H:%M
pattern when you intend hourly rotation.
regards, tom lane
Tom Lane wrote: >>> at logger startup or size-based >>>rotation, the rule would be to append. > > >>which then has a problem when you startup the postmaster after 10 hours >>of downtime ... hmmm. > > > Doesn't seem like a big problem --- at worst that logfile will get to be > double the size it normally would. ... continuing log entries with a time gap... > Note that this scheme effectively disables size-based rotation anyway, > unless you use one of the hacks we talked about like using a %H:%M > pattern when you intend hourly rotation. Please no colons in filenames (win32!) Regards, Andreas
On Saturday August 28 2004 9:30, Bruce Momjian wrote:
> Are we going to change this before beta2? I have not seen a final
> patch yet.
Attached is a revised patch:
+ changes postgresql.conf option 'log_filename_prefix' to 'log_filename',
and adds strftime() escape interpolation;
+ set default log_filename to 'postgresql-%Y-%m-%d_%H%M%S.log';
+ added postgresql.conf boolean option 'log_truncate_on_rotation', default
false;
+ log truncation is performed only on time-driven rotations and only when
log_truncate_on_rotation is true;
I did not add UTC offset logic nor logic to shift to top of the hour/day for
rotation periods of 60/1440 minutes, but would like to add that shortly if
time permits.
Ed
Attachment
"Ed L." <pgsql@bluepolka.net> writes:
> On Saturday August 28 2004 9:30, Bruce Momjian wrote:
>> Are we going to change this before beta2? I have not seen a final
>> patch yet.
> Attached is a revised patch:
Applied with minor revisions.
> I did not add UTC offset logic nor logic to shift to top of the hour/day for
> rotation periods of 60/1440 minutes, but would like to add that shortly if
> time permits.
I did the latter but not the former -- ie, rotation target times are
rounded off, but rounded with respect to GMT not local time. I didn't
see an obviously correct behavior of round-to-local-time across DST
transitions ...
regards, tom lane
On Monday August 30 2004 10:56, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > > > > Attached is a revised patch: > > Applied with minor revisions. > > > I did not add UTC offset logic nor logic to shift to top of the > > hour/day for rotation periods of 60/1440 minutes, but would like to add > > that shortly if time permits. > > I did the latter but not the former -- ie, rotation target times are > rounded off, but rounded with respect to GMT not local time. I didn't > see an obviously correct behavior of round-to-local-time across DST > transitions ... One idea for handling the round-to-localtime issue from the other end of the problem: optionally rotate logs upon an *interpolated* filename change. Then, 'server_log.%a' would cause a rotation when strftime() thinks it's midnight local, 'server_log.%H' would rotate at the top of the hour, etc. Possibly a half-baked idea. I also noticed pg_tm.tm_gmtoff is apparently not set, at least not for my local (US MT). Ed
"Ed L." <pgsql@bluepolka.net> writes:
> One idea for handling the round-to-localtime issue from the other end of the
> problem: optionally rotate logs upon an *interpolated* filename change.
The code-as-committed wants to be able to predict the time change in
advance. I'm not totally wedded to that, but I don't want to do a
strftime every time through the main loop, either ...
> I also noticed pg_tm.tm_gmtoff is apparently not set, at least not for my
> local (US MT).
It works for me. You want to look into why not for you?
regards, tom lane
Tom Lane wrote: > Andreas Pflug <pgadmin@pse-consulting.de> writes: > >>I don't have the time now to review the impact, but this might make >>interpreting the log filename difficult or impossible, effectively >>corrupting pg_logdir_ls. > > > So if you want to use that, you use a format that it can cope with. "you" is the backend, which should be able to interpret what it wrote. > I don't see a problem. Yes, you don't see a problem if the logfile can't be displayed on the client, I know that. My primary intention for contributing *any* logfile related stuff was to make it available through admin interfaces, and this goal seems to get obstructed in any possible way. Anybody volunteering to fix the pg_logdir_ls code at http://cvs.pgadmin.org/cgi-bin/viewcvs.cgi/pgadmin-tools/support/misc.c?rev=HEAD which should have been in the backend right from the start? What about misbehaving size rotation if the filename isn't unique? And what's a non human readable name.epoch pattern default good for? Regards, Andreas
Should the epoch snprintf format of the int64 pg_time_t timestamp be %lld instead of %d? Ed
On Tuesday August 31 2004 8:45, Ed L. wrote: > Should the epoch snprintf format of the int64 pg_time_t timestamp be %lld > instead of %d? Ah, I see you handled it.
On Monday August 30 2004 11:07, Ed L. wrote: > On Monday August 30 2004 10:56, Tom Lane wrote: > > "Ed L." <pgsql@bluepolka.net> writes: > > > Attached is a revised patch: > > > > Applied with minor revisions. > > > > > I did not add UTC offset logic nor logic to shift to top of the > > > hour/day for rotation periods of 60/1440 minutes, but would like to > > > add that shortly if time permits. > > > > I did the latter but not the former -- ie, rotation target times are > > rounded off, but rounded with respect to GMT not local time. I didn't > > see an obviously correct behavior of round-to-local-time across DST > > transitions ... This patch rotates logs on local time boundaries instead of UTC boundaries, e.g., midnight local for daily rotation instead of midnight UTC. It does so by parsing the "%z" result from strftime(). Correct me if I'm mistaken, but I *think* the correct behavior across DST transitions may be an orthogonal issue. Consider the case if one is truncating logs on rotation and rotating hourly. UTC vs local is irrelevant. If local time shifts backward from 02:00 to 01:00, our UTC offset will move in the negative direction. If 1) our policy were to truncate on rotation, and 2) we were rotating hourly or more frequently, and 3) our filename would be identical the 2nd time through that clock hour (i.e., it did not contain the epoch or UTC offset), this could cause a log file rotation into the same filename we just had open, thereby erasing an hour of log data. Apache's rotatelogs apparently has the same issue without a solution, and warns of it in the code. I am arguing for inclusion of this patch because 1) the utility of local time boundary rotations exceeds the risk for us, and because 2) the risk can be mitigated by a comment in the documentation and maybe postgresql.conf, and because 3) I think the issue already exists and this doesn't make it worse. Ed
Attachment
On Monday September 20 2004 4:43, Ed L. wrote: > > This patch rotates logs on local time boundaries instead of UTC > boundaries, e.g., midnight local for daily rotation instead of midnight > UTC. It does so by parsing the "%z" result from strftime(). > > ... I am arguing for > inclusion of this patch because 1) the utility of local time boundary > rotations exceeds the risk for us, and because 2) the risk can be > mitigated by a comment in the documentation and maybe postgresql.conf, > and because 3) I think the issue already exists and this doesn't make it > worse. And I'd add that working with UTC-oriented rotations in 8.0.0 beta code has already proved annoying and needlessly confusing, thus the patch. Daily log rotations occur at 18:00 here. Ed
"Ed L." <pgsql@bluepolka.net> writes:
> Consider the case if one is
> truncating logs on rotation and rotating hourly. UTC vs local is
> irrelevant. If local time shifts backward from 02:00 to 01:00, our UTC
> offset will move in the negative direction. If 1) our policy were to
> truncate on rotation, and 2) we were rotating hourly or more frequently,
> and 3) our filename would be identical the 2nd time through that clock hour
> (i.e., it did not contain the epoch or UTC offset), this could cause a log
> file rotation into the same filename we just had open, thereby erasing an
> hour of log data. Apache's rotatelogs apparently has the same issue
> without a solution, and warns of it in the code.
Hmm. Maybe we should remember the previous filename, and only truncate
when the new one is different (plus all the other conditions); else append.
regards, tom lane
On Monday September 20 2004 4:57, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > > Consider the case if one is > > truncating logs on rotation and rotating hourly. UTC vs local is > > irrelevant. If local time shifts backward from 02:00 to 01:00, our UTC > > offset will move in the negative direction. If 1) our policy were to > > truncate on rotation, and 2) we were rotating hourly or more > > frequently, and 3) our filename would be identical the 2nd time through > > that clock hour (i.e., it did not contain the epoch or UTC offset), > > this could cause a log file rotation into the same filename we just had > > open, thereby erasing an hour of log data. Apache's rotatelogs > > apparently has the same issue without a solution, and warns of it in > > the code. > > Hmm. Maybe we should remember the previous filename, and only truncate > when the new one is different (plus all the other conditions); else > append. Sounds good. If you accept that the DST gotcha already exists and this patch is independent of it, would you consider applying this patch regardless? I'd be happy to submit an addition for your idea as my time permits. Ed
"Ed L." <pgsql@bluepolka.net> writes:
> + /*
> + * We expect a strftime(%z) result of the form "[+-]HHMM" according to
> + * RFC822-conformant dates, where HH:MM is the unsigned UTC offset.
> + * If we don't get it, just return zero offset, and let the logs
> + * rotate on UTC time boundaries.
> + */
> +
> + count = strftime(msg, 6, "%z", localtime(&now));
That might work for you, but it's not portable.
regards, tom lane
On Monday September 20 2004 5:39, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > > + /* > > + * We expect a strftime(%z) result of the form "[+-]HHMM" > > according to + * RFC822-conformant dates, where HH:MM is the > > unsigned UTC offset. + * If we don't get it, just return zero > > offset, and let the logs + * rotate on UTC time boundaries. > > + */ > > + > > + count = strftime(msg, 6, "%z", localtime(&now)); > > That might work for you, but it's not portable. Bummer. Typo as well. Suggest a portable way to get the offset? Ed
On Monday September 20 2004 5:45, Ed L. wrote: > On Monday September 20 2004 5:39, Tom Lane wrote: > > "Ed L." <pgsql@bluepolka.net> writes: > > > + /* > > > + * We expect a strftime(%z) result of the form "[+-]HHMM" > > > according to + * RFC822-conformant dates, where HH:MM is the > > > unsigned UTC offset. + * If we don't get it, just return zero > > > offset, and let the logs + * rotate on UTC time boundaries. > > > + */ > > > + > > > + count = strftime(msg, 6, "%z", localtime(&now)); > > > > That might work for you, but it's not portable. Do you consider pg_tm.tm_gmtoff reliable and portable from pg_localtime(&now)? Ed
"Ed L." <pgsql@bluepolka.net> writes:
>> That might work for you, but it's not portable.
> Do you consider pg_tm.tm_gmtoff reliable and portable from
> pg_localtime(&now)?
Yeah, in fact I was just adapting the patch to use that.
regards, tom lane
On Monday September 20 2004 6:02, Tom Lane wrote: > "Ed L." <pgsql@bluepolka.net> writes: > >> That might work for you, but it's not portable. > > > > Do you consider pg_tm.tm_gmtoff reliable and portable from > > pg_localtime(&now)? > > Yeah, in fact I was just adapting the patch to use that. I have a 5-line check-last-filename patch, but it's so small you probably just want to add it yourself? Ed
"Ed L." <pgsql@bluepolka.net> writes:
> I have a 5-line check-last-filename patch, but it's so small you probably
> just want to add it yourself?
Yeah, it's pretty trivial. I just applied the attached.
regards, tom lane
Index: syslogger.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/syslogger.c,v
retrieving revision 1.8
diff -c -r1.8 syslogger.c
*** syslogger.c 31 Aug 2004 04:53:44 -0000 1.8
--- syslogger.c 21 Sep 2004 00:20:04 -0000
***************
*** 81,86 ****
--- 81,88 ----
static FILE *syslogFile = NULL;
+ static char *last_file_name = NULL;
+
/* These must be exported for EXEC_BACKEND case ... annoying */
#ifndef WIN32
int syslogPipe[2] = {-1, -1};
***************
*** 761,767 ****
else
filename = logfile_getname(time(NULL));
! if (Log_truncate_on_rotation && time_based_rotation)
fh = fopen(filename, "w");
else
fh = fopen(filename, "a");
--- 763,782 ----
else
filename = logfile_getname(time(NULL));
! /*
! * Decide whether to overwrite or append. We can overwrite if (a)
! * Log_truncate_on_rotation is set, (b) the rotation was triggered by
! * elapsed time and not something else, and (c) the computed file name
! * is different from what we were previously logging into.
! *
! * Note: during the first rotation after forking off from the postmaster,
! * last_file_name will be NULL. (We don't bother to set it in the
! * postmaster because it ain't gonna work in the EXEC_BACKEND case.)
! * So we will always append in that situation, even though truncating
! * would usually be safe.
! */
! if (Log_truncate_on_rotation && time_based_rotation &&
! last_file_name != NULL && strcmp(filename, last_file_name) != 0)
fh = fopen(filename, "w");
else
fh = fopen(filename, "a");
***************
*** 806,812 ****
set_next_rotation_time();
! pfree(filename);
}
--- 821,830 ----
set_next_rotation_time();
! /* instead of pfree'ing filename, remember it for next time */
! if (last_file_name != NULL)
! pfree(last_file_name);
! last_file_name = filename;
}
***************
*** 854,859 ****
--- 872,878 ----
set_next_rotation_time(void)
{
pg_time_t now;
+ struct pg_tm *tm;
int rotinterval;
/* nothing to do if time-based rotation is disabled */
***************
*** 863,875 ****
/*
* The requirements here are to choose the next time > now that is a
* "multiple" of the log rotation interval. "Multiple" can be interpreted
! * fairly loosely --- in particular, for intervals larger than an hour,
! * it might be interesting to align to local time instead of GMT.
*/
rotinterval = Log_RotationAge * 60; /* convert to seconds */
now = time(NULL);
now -= now % rotinterval;
now += rotinterval;
next_rotation_time = now;
}
--- 882,897 ----
/*
* The requirements here are to choose the next time > now that is a
* "multiple" of the log rotation interval. "Multiple" can be interpreted
! * fairly loosely. In this version we align to local time rather than
! * GMT.
*/
rotinterval = Log_RotationAge * 60; /* convert to seconds */
now = time(NULL);
+ tm = pg_localtime(&now);
+ now += tm->tm_gmtoff;
now -= now % rotinterval;
now += rotinterval;
+ now -= tm->tm_gmtoff;
next_rotation_time = now;
}