Thread: Fw: [BUGS] BUG #6011: Some extra messages are output in the event log at PostgreSQL startup

I'm sorry to interrupt you, but how should I treat this bug report I issued? 
Should I submit a bug fix patch for the latest source code (=v9.1)? I wish 
the fix will be back-patched in 8.3, too.

If yes, I would like to get a concensus on the solution. As I mentioned, I 
think that it is enough to just remove the two 
write_eventlog(EVENTLOG_INFORMATION_TYPE) calls. Although this may not be 
complete, it will be practical. The users feel it annoying that PostgreSQL 
always outputs informational (and perhaps useless) event log records at 
startup. As for errors, we can say "pg_ctl emits event logs when it 
encounters errors. This is a specification for troubleshooting. Currently, 
we cannot suppress error event logs emitted by pg_ctl according to your 
log_destination setting."

If the solution is okay, should I really submit a patch for just removing 
two lines? Or, can I expect that some committer will modify and commit the 
change directly?

Regards,
MauMau

From: ""MauMau"" <maumau307@gmail.com>
Sent: Saturday, May 07, 2011 9:35 AM
Subject: Re: [BUGS] BUG #6011: Some extra messages are output in the event 
log at PostgreSQL startup
>> Magnus Hagander <magnus@hagander.net> writes:
>>> Any events that happen before we have opened the regular logfile will
>>> be written to the eventlog if the server is running as a Windows
>>> service. There is no way to turn that off (other than removing the
>>> code and recompiling, of course). If we didn't send those to the
>>> eventlog, they would be completely lost since there is no stderr
>>> available to a windows service.
>>
>
> From: "Tom Lane" <tgl@sss.pgh.pa.us>
>> These particular messages appear to be coming from pg_ctl, not from the
>> server at all, so the server logging configuration is irrelevant anyway.
>>
>> What I think *is* relevant is pg_ctl's -s (--silent) switch, which is
>> defined as "only print errors, no informational messages".  I would
>> expect that to silence "waiting for ..." messages, and indeed it appears
>> to do so in all the cases on the Unix side of the fence.  However,
>> someone chose to code these Windows-specific messages as direct
>> write_eventlog calls instead of going through print_msg, which means
>> that -s doesn't silence them.  So possibly the OP is right that this is
>> a bug and not just an unimplemented feature.
>
> I think that those two messages in question are not very helpful, so I 
> hope those are omitted by default (i.e. even without pg_ctl's -s). If they 
> are useful and should be logged in the event log, why aren't they logged 
> in syslog on UNIX/Linux?
>
> So, I wish that the direct calls to write_eventlog will be eliminated. If 
> those messages are useful for some reason, it may be okay that pg_ctl's -s 
> hides them at worst. However, it's better to remove the write_eventlog 
> calls in question, because those software dependent on PostgreSQL have to 
> be modified to add -s.
>
> Can I expect this will be treated as a bug and fixed in 8.3.x too? BTW, 
> how can I check the bug treatment? I saw somewhere that you don't like 
> issue trackers such as JIRA or Bugzilla.
>

"MauMau" <maumau307@gmail.com> wrote:
> I'm sorry to interrupt you, but how should I treat this bug report
> I issued?  Should I submit a bug fix patch for the latest source
> code (=v9.1)?
Patches should always be submitted against the HEAD of the master
branch.
> I wish the fix will be back-patched in 8.3, too.
I guess the question is whether this is a bug which causes more
problems than the potential breakage which might ensue for someone
who relies on the current behavior.  How sure can you be that nobody
relies on seeing those messages?  No information (like a history of
database start times) is lost without these entries?
> If yes, I would like to get a concensus on the solution. As I
> mentioned, I think that it is enough to just remove the two 
> write_eventlog(EVENTLOG_INFORMATION_TYPE) calls.
Someone running at the command line would still get this feedback,
right?
> If the solution is okay, should I really submit a patch for just
> removing two lines?
That's the normal way to propose a change; even a small one.
> Or, can I expect that some committer will modify and commit the 
> change directly?
That might happen.  Your odds are better if you propose something
that a committer can review.  Most of the committers don't have
Windows readily available, so it would also boost your chances to
report on what testing you've done -- running at the command line
and as a service, what Windows version(s), etc.
-Kevin

On Thu, May 12, 2011 at 2:57 PM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
>> I wish the fix will be back-patched in 8.3, too.
>
> I guess the question is whether this is a bug which causes more
> problems than the potential breakage which might ensue for someone
> who relies on the current behavior.  How sure can you be that nobody
> relies on seeing those messages?  No information (like a history of
> database start times) is lost without these entries?

I think Tom had the right idea upthread: what we should do is make the
"-s" option to pg_ctl suppress these messages (as it does with similar
messages on Linux).  Removing them altogether seems like overkill, for
the reasons you mention.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

From: "Robert Haas" <robertmhaas@gmail.com>
On Thu, May 12, 2011 at 2:57 PM, Kevin Grittner
<Kevin.Grittner@wicourts.gov> wrote:
>>> I wish the fix will be back-patched in 8.3, too.
>>
>> I guess the question is whether this is a bug which causes more
>> problems than the potential breakage which might ensue for someone
>> who relies on the current behavior. How sure can you be that nobody
>> relies on seeing those messages? No information (like a history of
>> database start times) is lost without these entries?

> I think Tom had the right idea upthread: what we should do is make the
> "-s" option to pg_ctl suppress these messages (as it does with similar
> messages on Linux).  Removing them altogether seems like overkill, for
> the reasons you mention.


Thank you, Magnus, Tom, Dave, Kevin, and Robert. Maybe I could get a 
consensus of opinion on the treatment of bug #6011. The summary is shown 
below. However, as mentioned previously, there are some concerns. So I'll 
wait for more opinions for a week, then write and test a patch, and submit 
it as a reply to this mail thread.


How to treat
========================================
Treat it as a bug. (I hope some committer will kindly back-patch to older 
versions.)

Make pg_ctl's -s option suppress informational event logging. The 
modifications are:

1. write_event_log() (pg_ctl.c)
If "-s" was specified and the level argument is EVENTLOG_INFORMATION_TYPE, 
just return without doing anything.

2. pgwin32_CommandLine() (pg_ctl.c)
If "-s" was specified when running "pg_ctl register", add "-s" to "pg_ctl 
runservice" command line built in this function.


Concerns
========================================
Existing software which use PostgreSQL needs to be modified to add -s to 
pg_ctl. Moreover, pg_ctl unregister and pg_ctl register must be performed to 
make the patch effective in existing installations.

The two messages in question may be just annoying to users, and they might 
want those messages to disappear without -s. They claim that it is 
inconsistent that those messages are not recorded in syslog on UNIX/Linux.

As described in "How to treat", the PostgreSQL Windows service must be 
registered by "pg_ctl register -s" to make use of this patch. However, 
according to the current manual, "pg_ctl register" does not take -s option. 
Actually, pg_ctl does not refuse to take -s, so this is not a big problem.

pg_ctl register [-N servicename] [-U username] [-P password] [-D datadir] 
[-w] [-t seconds] [-o options]
Regards,
MauMau

"MauMau" <maumau307@gmail.com> wrote:
> From: "Robert Haas" <robertmhaas@gmail.com>
>> I think Tom had the right idea upthread: what we should do is
>> make the "-s" option to pg_ctl suppress these messages (as it
>> does with similar messages on Linux).  Removing them altogether
>> seems like overkill, for the reasons you mention.
Agreed.
> So I'll wait for more opinions for a week, then write and test a
> patch, and submit it as a reply to this mail thread.
Sounds reasonable.
> Treat it as a bug. (I hope some committer will kindly back-patch
> to older versions.)
> 
> Make pg_ctl's -s option suppress informational event logging.
This will ultimately be up to a committer (and I'm not one), but to
me it seems reasonable to back-patch if it is addressed this way.
> Existing software which use PostgreSQL needs to be modified to add
> -s to pg_ctl. Moreover, pg_ctl unregister and pg_ctl register must
> be performed to make the patch effective in existing
> installations.
That is probably a very *good* thing if you want it to be considered
for back-patch.  Having a bug fix release arbitrarily change
existing behavior which isn't data-destroying or a security risk is
not a good thing and is contrary to PostgreSQL policy for
maintaining stable branches.  We can't know who might be, for
example, pulling such messages out of their logs for reporting or
monitoring purposes.  If we made changes that can conceivably break
things on applying bug fix releases, we would have fewer people
applying them, and that would be bad for everyone.
> The two messages in question may be just annoying to users, and
> they might want those messages to disappear without -s. They claim
> that it is inconsistent that those messages are not recorded in
> syslog on UNIX/Linux.
I can only dream of what it's like to work somewhere that fussing
over two informational log messages on an infrequent event like
restarting a database (that *is* an infrequent event, right?) would
be something I had time for.  They are very fortunate people to be
in such a position.  It would appear that finding the time to add
the -s switch shouldn't be too hard in such an environment.
> the PostgreSQL Windows service must be registered by "pg_ctl
> register -s" to make use of this patch. However, according to the
> current manual, "pg_ctl register" does not take -s option.
> Actually, pg_ctl does not refuse to take -s, so this is not a big
> problem.
> 
> pg_ctl register [-N servicename] [-U username] [-P password]
> [-D datadir] [-w] [-t seconds] [-o options]
When you write the patch, be sure to include a fix for the docs
here, please.
Thanks for taking the time to work through the issue.
-Kevin

From: "Kevin Grittner" <Kevin.Grittner@wicourts.gov>
> "MauMau" <maumau307@gmail.com> wrote:
>> Make pg_ctl's -s option suppress informational event logging.
>
> This will ultimately be up to a committer (and I'm not one), but to
> me it seems reasonable to back-patch if it is addressed this way.
>
>
>> the PostgreSQL Windows service must be registered by "pg_ctl
>> register -s" to make use of this patch. However, according to the
>> current manual, "pg_ctl register" does not take -s option.
>> Actually, pg_ctl does not refuse to take -s, so this is not a big
>> problem.
>>
>> pg_ctl register [-N servicename] [-U username] [-P password]
>> [-D datadir] [-w] [-t seconds] [-o options]
>
> When you write the patch, be sure to include a fix for the docs
> here, please.
>

I attached a patch to fix this bug. I performed the following tests
successfully on Windows Vista (32-bit).

[test case 1]
pg_ctl register -s ...
start PostgreSQL as Windows service from the Control Panel
[result]
The following two messages in question disappeared in event log:

Waiting for server startup...
Server started and accepting connections

[test case 2]
pg_ctl register -s ...
start PostgreSQL not as Windows service (pg_ctl start -w)
[result]
The above messages didn't appear in event log (the same behavior as before)

[test case 3]
pg_ctl register ... (without -s)
start PostgreSQL as Windows service from the Control Panel
[result]
The above messages appeared in event log (the same behavior as before)

[test case 4]
pg_ctl register ... (without -s)
start PostgreSQL not as Windows service (pg_ctl start -w)
[result]
The above messages didn't appear in event log (the same behavior as before)

I wish this will be back-patched in the next minor release.

Regards
MauMau

On Mon, May 23, 2011 at 16:49, MauMau <maumau307@gmail.com> wrote:
> From: "Kevin Grittner" <Kevin.Grittner@wicourts.gov>
>>
>> "MauMau" <maumau307@gmail.com> wrote:
>>>
>>> Make pg_ctl's -s option suppress informational event logging.
>>
>> This will ultimately be up to a committer (and I'm not one), but to
>> me it seems reasonable to back-patch if it is addressed this way.
>>
>>
>>> the PostgreSQL Windows service must be registered by "pg_ctl
>>> register -s" to make use of this patch. However, according to the
>>> current manual, "pg_ctl register" does not take -s option.
>>> Actually, pg_ctl does not refuse to take -s, so this is not a big
>>> problem.
>>>
>>> pg_ctl register [-N servicename] [-U username] [-P password]
>>> [-D datadir] [-w] [-t seconds] [-o options]
>>
>> When you write the patch, be sure to include a fix for the docs
>> here, please.
>>
>
> I attached a patch to fix this bug. I performed the following tests
> successfully on Windows Vista (32-bit).
>
<snip test cases>

> I wish this will be back-patched in the next minor release.

Thanks, sorry about the delay, patch applied. I backpatched it back to
8.3 which is as far as it applied cleanly - I'm not excited enough
about it to bother a manual backpatch to 8.2.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

From: "Magnus Hagander" <magnus@hagander.net>
Thanks, sorry about the delay, patch applied. I backpatched it back to
8.3 which is as far as it applied cleanly - I'm not excited enough
about it to bother a manual backpatch to 8.2.

Thank you very much, Magnus. I don't mind 8.2 as >= 8.3 is enough for me.

Regards
MauMau