Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
"Magnus Hagander" <[EMAIL PROTECTED]> writes:
> Here is an updated patch, that should take care of this. Tested that it
> solves the problem reported.
I compared this to the version Bruce applied earlier and decided that
his version was good. I don't think we should change the original logic
that treated write_syslogger_file as an independent special destination
for the syslogger process only.
I've backpatched that version to 8.0 branch.
>> If the logger is complaining, it's quite possibly because it's
>> unable to
>> write to its file. Now that you mention it, doesn't this code go into
>> infinite recursion if write_syslogger_file_binary() tries to ereport?
> I haven't looked at this part, it appears a separate (but closely
> related) issue.
Actually, your change to make write_syslogger_file_binary() use
write_stderr seems like a fine solution to this problem. However
you didn't get it quite right: the call has to be more like
/* can't use ereport here because of possible recursion */
if (rc != count)
write_stderr("could not write to log file: %s\n", strerror(errno));
since write_stderr doesn't know about %m and doesn't supply a free
newline.
Applied and backpatched to 8.0.
regards, tom lane
---(end of broadcast)---
TIP 3: 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] [BUGS] BUG #1466: #maintenace_work_mem = 16384
Actually, I'd like it considered for 8.0.X as well. Right now if yuo change the config file in a way that it won't load, you get no error message. Which is cearly not very good. I'll certainly accept if you think it's not safe enough for 8.0.x, but please consider it. //Magnus >-Original Message- >From: Bruce Momjian [mailto:[EMAIL PROTECTED] >Sent: den 27 februari 2005 02:04 >To: Magnus Hagander >Cc: Tom Lane; PostgreSQL-patches; Andreas Pflug >Subject: Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384 > > > >Patch applied. Thanks. > >I assume this is not for 8.0.X. > >--- > > > >Magnus Hagander wrote: >> >>> The proposed test on Redirect_stderr looks pretty fishy >too; for one >> >>> thing it will almost certainly not be the right thing >> >inside the stderr >> >>> logger subprocess itself. >> > >> >> Could you explain further what the issue is there? >> > >> >Inside the logger subprocess, Redirect_stderr is guaranteed >true (since >> >it'll be inherited from the postmaster) and therefore the proposed >> >change ensures that anything the logger might want to complain about >> >goes to the original stderr, ie, into the bit bucket rather than >> >someplace useful. Perhaps something like >> > >> >if ((!Redirect_stderr || am_syslogger) && pgwin32_is_service()) >> > >> >would be reasonable. >> >> >> >> Here is an updated patch, that should take care of this. >Tested that it >> solves the problem reported. >> >> >> >> There is special code in the send_message_to_server_log >> >function to make >> >> sure it's written directly to the file. >> > >> >If the logger is complaining, it's quite possibly because it's >> >unable to >> >write to its file. Now that you mention it, doesn't this >code go into >> >infinite recursion if write_syslogger_file_binary() tries >to ereport? >> > >> >> I haven't looked at this part, it appears a separate (but closely >> related) issue. >> >> Perhaps Andreas can comment on this? >> >> //Magnus > >Content-Description: stderr.patch > >[ Attachment, skipping... ] > >> >> ---(end of >broadcast)--- >> TIP 5: Have you checked our extensive FAQ? >> >>http://www.postgresql.org/docs/faq > >-- > 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 > ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
Patch applied. Thanks. I assume this is not for 8.0.X. --- Magnus Hagander wrote: > >>> The proposed test on Redirect_stderr looks pretty fishy too; for one > >>> thing it will almost certainly not be the right thing > >inside the stderr > >>> logger subprocess itself. > > > >> Could you explain further what the issue is there? > > > >Inside the logger subprocess, Redirect_stderr is guaranteed true (since > >it'll be inherited from the postmaster) and therefore the proposed > >change ensures that anything the logger might want to complain about > >goes to the original stderr, ie, into the bit bucket rather than > >someplace useful. Perhaps something like > > > > if ((!Redirect_stderr || am_syslogger) && pgwin32_is_service()) > > > >would be reasonable. > > > > Here is an updated patch, that should take care of this. Tested that it > solves the problem reported. > > > >> There is special code in the send_message_to_server_log > >function to make > >> sure it's written directly to the file. > > > >If the logger is complaining, it's quite possibly because it's > >unable to > >write to its file. Now that you mention it, doesn't this code go into > >infinite recursion if write_syslogger_file_binary() tries to ereport? > > > > I haven't looked at this part, it appears a separate (but closely > related) issue. > > Perhaps Andreas can comment on this? > > //Magnus Content-Description: stderr.patch [ Attachment, skipping... ] > > ---(end of broadcast)--- > TIP 5: Have you checked our extensive FAQ? > >http://www.postgresql.org/docs/faq -- 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 ---(end of broadcast)--- TIP 3: 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] [BUGS] BUG #1466: #maintenace_work_mem = 16384
>>> The proposed test on Redirect_stderr looks pretty fishy too; for one >>> thing it will almost certainly not be the right thing >inside the stderr >>> logger subprocess itself. > >> Could you explain further what the issue is there? > >Inside the logger subprocess, Redirect_stderr is guaranteed true (since >it'll be inherited from the postmaster) and therefore the proposed >change ensures that anything the logger might want to complain about >goes to the original stderr, ie, into the bit bucket rather than >someplace useful. Perhaps something like > > if ((!Redirect_stderr || am_syslogger) && pgwin32_is_service()) > >would be reasonable. Here is an updated patch, that should take care of this. Tested that it solves the problem reported. >> There is special code in the send_message_to_server_log >function to make >> sure it's written directly to the file. > >If the logger is complaining, it's quite possibly because it's >unable to >write to its file. Now that you mention it, doesn't this code go into >infinite recursion if write_syslogger_file_binary() tries to ereport? > I haven't looked at this part, it appears a separate (but closely related) issue. Perhaps Andreas can comment on this? //Magnus stderr.patch Description: stderr.patch ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
"Magnus Hagander" <[EMAIL PROTECTED]> writes: >> I dislike going through write_stderr because it requires an extra, >> useless gettext() call, > It does? First thing it does. > Isn't buf.data already "gettextified"? Yeah; the gettext() call is against the "%s". (If it did gettext() on the buffer data it would be outright wrong.) Useless, but potentially a cycle-sucker under high log volume. >> but you can't assume that every log message the system ever prints >> is going to be under 2K. > We're not assuming that! Yes you are (and no the "it's user error" argument doesn't impress me). It's trivial to work around both these problems anyway; just a matter of repeating the logic in-line instead of trying to use write_stderr. The issue that's actually bothering me is: >> The proposed test on Redirect_stderr looks pretty fishy too; for one >> thing it will almost certainly not be the right thing inside the stderr >> logger subprocess itself. > Could you explain further what the issue is there? Inside the logger subprocess, Redirect_stderr is guaranteed true (since it'll be inherited from the postmaster) and therefore the proposed change ensures that anything the logger might want to complain about goes to the original stderr, ie, into the bit bucket rather than someplace useful. Perhaps something like if ((!Redirect_stderr || am_syslogger) && pgwin32_is_service()) would be reasonable. > There is special code in the send_message_to_server_log function to make > sure it's written directly to the file. If the logger is complaining, it's quite possibly because it's unable to write to its file. Now that you mention it, doesn't this code go into infinite recursion if write_syslogger_file_binary() tries to ereport? regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
>> What do you think of the following as a fix? > >I dislike going through write_stderr because it requires an extra, >useless gettext() call, It does? Isn't buf.data already "gettextified"? write_stderr just calls vfprintf. Or is this an example of me not knowing how gettext works? > and because in the Windows case it imposes > an arbitrary length limit that is certain to be exceeded. > The length >limit is probably OK for the existing limited usages of write_stderr, >but you can't assume that every log message the system ever prints >is going to be under 2K. We're not assuming that! If the system is configured to log to eventlog, this code is never called. If the system is configured to log to a file (redirect_stderr), this code is never called. It is *only* called when: - System has not read it's full configuration yet (which is the case we are trying to address) - System is configured for logging to stderr *without* capturing it to a file *and* running as a service. I'd argue this is a configuration error. >The proposed test on Redirect_stderr looks pretty fishy too; for one >thing it will almost certainly not be the right thing inside the stderr >logger subprocess itself. Could you explain further what the issue is there? There is special code in the send_message_to_server_log function to make sure it's written directly to the file. //Magnus ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
"Magnus Hagander" <[EMAIL PROTECTED]> writes: > What do you think of the following as a fix? I dislike going through write_stderr because it requires an extra, useless gettext() call, and because in the Windows case it imposes an arbitrary length limit that is certain to be exceeded. The length limit is probably OK for the existing limited usages of write_stderr, but you can't assume that every log message the system ever prints is going to be under 2K. The proposed test on Redirect_stderr looks pretty fishy too; for one thing it will almost certainly not be the right thing inside the stderr logger subprocess itself. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] [BUGS] BUG #1466: #maintenace_work_mem = 16384
>> Can defaults be set to the result of a function? > >By "power-on default" I mean the value that the variable is assigned as >its static initializer. That has to be a link-time constant (at least >in C ...). Anything else will mean misbehavior if an error occurs >before the GUC code itself has been able to initialize. Hmm. Ok. >> The problem with changing the default from stderr to >eventlog is that it >> will send the messages to the eventlog even if running from >the console, >> which is not good. > >It's not perfect maybe, but it's certainly less bad than the present >behavior. At least we know that the eventlog will always exist on >Windows. I'm not sure how interesting the running-from-the-console >case actually is for Windows users anyway. It's definitly interesting when debugging :-) What do you think of the following as a fix? It should put the message in the eventlog when running as a service, but stderr when on the console. //Magnus stderr.patch Description: stderr.patch ---(end of broadcast)--- TIP 8: explain analyze is your friend
