Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-21 Thread Magnus Hagander
On Sun, Jul 20, 2014 at 7:44 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Fri, Jul 18, 2014 at 7:08 PM, MauMau maumau...@gmail.com wrote:

 From: Magnus Hagander mag...@hagander.net

 On Fri, Jul 18, 2014 at 5:33 AM, Amit Kapila amit.kapil...@gmail.com
 wrote:

 On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander mag...@hagander.net
 wrote:


 Did anyone actually test this patch? :)

 I admit I did not build it on Windows specifically because I assumed
 that was done as part of the development and review. And the changes
 to pg_event.c can never have built, since the file does not include
 the required header.


 I have tested it on Windows and infact on Linux as well to see if there
 is any side impact before marking it as Ready For Committer.

 It seems to me that the required header is removed in last version
 (pg_ctl_eventsrc_v11) where MessageBox() related changes have been
 removed from patch as per recent discussion.  Sorry for not being able
 to check last version posted.


 Gotcha. Thanks for clarifying, and I apologize if I came across a bit
 harsh even with the smiley.

 The statement was not at all harsh.  I think you are right in asking that
 question and I believe that is the minimum expectation once the patch
 reaches Ready To Committer stage.



 I'm sorry to have caused both of you trouble.  I have to admit that I
 didn't compile the source when I removed the MessageBox()-related changes.
 The attached patch fixes that.  I confirmed successful build this time.

 I have tested the attached patch on windows, it works fine both for
 default and non-default cases.  It passes other regression tests as well
 and build went fine on Linux.

 One more thing about inclusion of new header file in pgevent.c,  I think
 that is okay because we include it in other modules (client side) present
 in bin directory like reindex.

Thanks to you both for confirming it works, applied in the current state.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-19 Thread Amit Kapila
On Fri, Jul 18, 2014 at 7:08 PM, MauMau maumau...@gmail.com wrote:

 From: Magnus Hagander mag...@hagander.net

 On Fri, Jul 18, 2014 at 5:33 AM, Amit Kapila amit.kapil...@gmail.com
wrote:

 On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander mag...@hagander.net
 wrote:


 Did anyone actually test this patch? :)

 I admit I did not build it on Windows specifically because I assumed
 that was done as part of the development and review. And the changes
 to pg_event.c can never have built, since the file does not include
 the required header.


 I have tested it on Windows and infact on Linux as well to see if there
 is any side impact before marking it as Ready For Committer.

 It seems to me that the required header is removed in last version
 (pg_ctl_eventsrc_v11) where MessageBox() related changes have been
 removed from patch as per recent discussion.  Sorry for not being able
 to check last version posted.


 Gotcha. Thanks for clarifying, and I apologize if I came across a bit
 harsh even with the smiley.

The statement was not at all harsh.  I think you are right in asking that
question and I believe that is the minimum expectation once the patch
reaches Ready To Committer stage.


 I'm sorry to have caused both of you trouble.  I have to admit that I
didn't compile the source when I removed the MessageBox()-related changes.
 The attached patch fixes that.  I confirmed successful build this time.

I have tested the attached patch on windows, it works fine both for
default and non-default cases.  It passes other regression tests as well
and build went fine on Linux.

One more thing about inclusion of new header file in pgevent.c,  I think
that is okay because we include it in other modules (client side) present
in bin directory like reindex.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-18 Thread Magnus Hagander
On Fri, Jul 18, 2014 at 5:33 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander mag...@hagander.net
 wrote:

 Did anyone actually test this patch? :)

 I admit I did not build it on Windows specifically because I assumed
 that was done as part of the development and review. And the changes
 to pg_event.c can never have built, since the file does not include
 the required header.

 I have tested it on Windows and infact on Linux as well to see if there
 is any side impact before marking it as Ready For Committer.

 It seems to me that the required header is removed in last version
 (pg_ctl_eventsrc_v11) where MessageBox() related changes have been
 removed from patch as per recent discussion.  Sorry for not being able
 to check last version posted.

Gotcha. Thanks for clarifying, and I apologize if I came across a bit
harsh even with the smiley.


 I have reverted that part of the patch for now, hopefully that'll
 unbreak the buildfarm.

 Do you want me to write a patch to use DEFAULT_EVENT_SOURCE in
 pgevent?

I'm not sure it's worth it. I do like the property that pg_event
doesn't have to pull in the full set of headers. But I guess it's
quite confusing to have *one* place that doesn't use the #define. So I
guess if it does work to just pull int he required header then yes, we
should do it - but if it turns out that header causes any conflicts
with anything else we're doing in the file, then let's just skipp it.


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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-18 Thread MauMau

From: Magnus Hagander mag...@hagander.net
On Fri, Jul 18, 2014 at 5:33 AM, Amit Kapila amit.kapil...@gmail.com 
wrote:

On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander mag...@hagander.net
wrote:


Did anyone actually test this patch? :)

I admit I did not build it on Windows specifically because I assumed
that was done as part of the development and review. And the changes
to pg_event.c can never have built, since the file does not include
the required header.


I have tested it on Windows and infact on Linux as well to see if there
is any side impact before marking it as Ready For Committer.

It seems to me that the required header is removed in last version
(pg_ctl_eventsrc_v11) where MessageBox() related changes have been
removed from patch as per recent discussion.  Sorry for not being able
to check last version posted.


Gotcha. Thanks for clarifying, and I apologize if I came across a bit
harsh even with the smiley.


I'm sorry to have caused both of you trouble.  I have to admit that I didn't 
compile the source when I removed the MessageBox()-related changes.  The 
attached patch fixes that.  I confirmed successful build this time.


Thank you for committing, Magnus-san, and thank you very much for kind and 
repeated review and help, Amit-san.



Regards
MauMau


pgevent.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-17 Thread Magnus Hagander
On Wed, Jul 16, 2014 at 2:01 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 So as a conclusion, the left over items to be handled for patch are:

 1. Remove the new usage related to use of same event source name
 for registration from pgevent.
 2. Document the information to prevent loss of messages in some
 scenarios as explained above.


 I noticed the continued discussion after I had prepared and submitted the
 revised patch.  OK, how about the patch in the previous mail, Magnus-san?

I mean that the -e option is valid for *all* commands, not just
register/unregister. If you include it in register, pg_ctl will write
it to the commandline used for start -- so clearly it is valid for the
start command as well. And it is certainly possible for a completely
different service to run pg_ctl start, in which case it will also be
used.

I have updated the patch with this change, so please verify that it
still works. I also rewrote the documentation slightly.

With that, applied. Thanks!

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-17 Thread Magnus Hagander
On Thu, Jul 17, 2014 at 12:45 PM, Magnus Hagander mag...@hagander.net wrote:
 On Wed, Jul 16, 2014 at 2:01 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 So as a conclusion, the left over items to be handled for patch are:

 1. Remove the new usage related to use of same event source name
 for registration from pgevent.
 2. Document the information to prevent loss of messages in some
 scenarios as explained above.


 I noticed the continued discussion after I had prepared and submitted the
 revised patch.  OK, how about the patch in the previous mail, Magnus-san?

 I mean that the -e option is valid for *all* commands, not just
 register/unregister. If you include it in register, pg_ctl will write
 it to the commandline used for start -- so clearly it is valid for the
 start command as well. And it is certainly possible for a completely
 different service to run pg_ctl start, in which case it will also be
 used.

 I have updated the patch with this change, so please verify that it
 still works. I also rewrote the documentation slightly.

 With that, applied. Thanks!

Did anyone actually test this patch? :)

I admit I did not build it on Windows specifically because I assumed
that was done as part of the development and review. And the changes
to pg_event.c can never have built, since the file does not include
the required header.

I have reverted that part of the patch for now, hopefully that'll
unbreak the buildfarm.

For the time being at least I left the other changes to DEFAULT_EVENT_SOURCE in.


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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-17 Thread Amit Kapila
On Thu, Jul 17, 2014 at 4:51 PM, Magnus Hagander mag...@hagander.net
wrote:

 Did anyone actually test this patch? :)

 I admit I did not build it on Windows specifically because I assumed
 that was done as part of the development and review. And the changes
 to pg_event.c can never have built, since the file does not include
 the required header.

I have tested it on Windows and infact on Linux as well to see if there
is any side impact before marking it as Ready For Committer.

It seems to me that the required header is removed in last version
(pg_ctl_eventsrc_v11) where MessageBox() related changes have been
removed from patch as per recent discussion.  Sorry for not being able
to check last version posted.

 I have reverted that part of the patch for now, hopefully that'll
 unbreak the buildfarm.

Do you want me to write a patch to use DEFAULT_EVENT_SOURCE in
pgevent?


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread Magnus Hagander
On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Tue, Jul 15, 2014 at 8:57 PM, MauMau maumau...@gmail.com wrote:

 From: Magnus Hagander mag...@hagander.net

 Well, it does in a couple of places. I'm nto sure it's that important
 (as nobody has AFAIK ever requested that change from for example EDB),
 but it's not a bad thing.

 I think this is nothing specific to any vendor rather it's good to have
 a define when it is used at multiple places.

Sure, I don't object to the change itself, but the motivation was strange.

There's also the change to throw an error if the source is already
registered, which is potentially a bigger problem. Since the default
will be the same everywhere, do we really want to throw an error when
you install a second version, now that this is the normal state?

There's also definitely a problem in that that codepath fires up a
MessageBox, but it's just a function called in a DLL. It might just as
well be called from a background service or from within an installer
with no visible desktop, at which point the process will appear to be
hung... I'm pretty sure you're not allowed to do that.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread Amit Kapila
On Wed, Jul 16, 2014 at 2:11 PM, Magnus Hagander mag...@hagander.net
wrote:
 On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila amit.kapil...@gmail.com
wrote:

 There's also the change to throw an error if the source is already
 registered, which is potentially a bigger problem.

I think generally if the s/w is installed/registered and we try to install
it
second time without un-installing previous one, it gives some notice
indicating the same.

 Since the default
 will be the same everywhere, do we really want to throw an error when
 you install a second version, now that this is the normal state?

Allowing second version to overwrite the first can also cause problems
similar to what is reported in this thread, basically what if the second
installation/registration is removed, won't it cause the first one to loose
messages?


 There's also definitely a problem in that that codepath fires up a
 MessageBox, but it's just a function called in a DLL.

There are other paths in the same code which already fires up
MessageBox.

 It might just as
 well be called from a background service or from within an installer
 with no visible desktop, at which point the process will appear to be
 hung... I'm pretty sure you're not allowed to do that.

So in that case shouldn't we get rid of MessageBox() or provide
alternate way of logging from pgevent module.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread Magnus Hagander
On Wed, Jul 16, 2014 at 12:31 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Wed, Jul 16, 2014 at 2:11 PM, Magnus Hagander mag...@hagander.net
 wrote:
 On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila amit.kapil...@gmail.com
 wrote:

 There's also the change to throw an error if the source is already
 registered, which is potentially a bigger problem.

 I think generally if the s/w is installed/registered and we try to install
 it
 second time without un-installing previous one, it gives some notice
 indicating the same.

 Since the default
 will be the same everywhere, do we really want to throw an error when
 you install a second version, now that this is the normal state?

 Allowing second version to overwrite the first can also cause problems
 similar to what is reported in this thread, basically what if the second
 installation/registration is removed, won't it cause the first one to loose
 messages?

It will, this is true. I'm not sure there's a good way around that
given now Windows Event Logs work though, unless we restrict usage a
lot (as in only one installation of postgres at a time for example). I
thnk it's better to document what you need to do in a case like this
(re-register the existing DLL).


 There's also definitely a problem in that that codepath fires up a
 MessageBox, but it's just a function called in a DLL.

 There are other paths in the same code which already fires up
 MessageBox.

Ouch, I didn't realize that - I just looked at the patch. What's worse
is it's probably written by me :)

However, I'd say the one being added here is much more likely to fire
under such circumstances. Of the existing ones, the only really likely
case for them to fire is a permission denied case, and that will most
likely only happen from an interactive session. That might be why we
haven't seen it...

 It might just as
 well be called from a background service or from within an installer
 with no visible desktop, at which point the process will appear to be
 hung... I'm pretty sure you're not allowed to do that.

 So in that case shouldn't we get rid of MessageBox() or provide
 alternate way of logging from pgevent module.

It's something we might want to consider, but let's consider that a
separate patch.

Actually, it surprises me that we haven't had an issue before. But I
guess maybe the installers don't actually use regsvr32, but instead
just registers it manually by sticking it in the registry?


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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread Amit Kapila
On Wed, Jul 16, 2014 at 4:06 PM, Magnus Hagander mag...@hagander.net
wrote:

 On Wed, Jul 16, 2014 at 12:31 PM, Amit Kapila amit.kapil...@gmail.com
wrote:
  On Wed, Jul 16, 2014 at 2:11 PM, Magnus Hagander mag...@hagander.net
  wrote:
  On Wed, Jul 16, 2014 at 6:37 AM, Amit Kapila amit.kapil...@gmail.com
  wrote:
 
  There's also the change to throw an error if the source is already
  registered, which is potentially a bigger problem.
 
  I think generally if the s/w is installed/registered and we try to
install
  it
  second time without un-installing previous one, it gives some notice
  indicating the same.
 
  Since the default
  will be the same everywhere, do we really want to throw an error when
  you install a second version, now that this is the normal state?
 
  Allowing second version to overwrite the first can also cause problems
  similar to what is reported in this thread, basically what if the second
  installation/registration is removed, won't it cause the first one to
loose
  messages?

 It will, this is true. I'm not sure there's a good way around that
 given now Windows Event Logs work though, unless we restrict usage a
 lot (as in only one installation of postgres at a time for example). I
 thnk it's better to document what you need to do in a case like this
 (re-register the existing DLL).

Given that now user has a way to use separate names for event log
messages, I think it is better not to change default behaviour and rather
just document the same.

  There's also definitely a problem in that that codepath fires up a
  MessageBox, but it's just a function called in a DLL.
 
  There are other paths in the same code which already fires up
  MessageBox.

 Ouch, I didn't realize that - I just looked at the patch. What's worse
 is it's probably written by me :)

 However, I'd say the one being added here is much more likely to fire
 under such circumstances. Of the existing ones, the only really likely
 case for them to fire is a permission denied case, and that will most
 likely only happen from an interactive session. That might be why we
 haven't seen it...

  It might just as
  well be called from a background service or from within an installer
  with no visible desktop, at which point the process will appear to be
  hung... I'm pretty sure you're not allowed to do that.
 
  So in that case shouldn't we get rid of MessageBox() or provide
  alternate way of logging from pgevent module.

 It's something we might want to consider, but let's consider that a
 separate patch.

Sure, that make sense.

So as a conclusion, the left over items to be handled for patch are:
1. Remove the new usage related to use of same event source name
for registration from pgevent.
2. Document the information to prevent loss of messages in some
scenarios as explained above.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread MauMau

From: Magnus Hagander mag...@hagander.net

There's also the change to throw an error if the source is already
registered, which is potentially a bigger problem. Since the default
will be the same everywhere, do we really want to throw an error when
you install a second version, now that this is the normal state?

There's also definitely a problem in that that codepath fires up a
MessageBox, but it's just a function called in a DLL. It might just as
well be called from a background service or from within an installer
with no visible desktop, at which point the process will appear to be
hung... I'm pretty sure you're not allowed to do that.


I got what you mean.  I removed changes in pgevent.c except for the default 
name.  I attached the revised patch.




More importantly, isn't it wrong to claim it will only be used for
register and unregister? If we get an early failure in start, for
example, there are numerous codepaths that will end up calling
write_stderr(), which will use the eventlog when running as a service.
Shouldn't the -e parameter be moved under common options?



Yes, you are right.  -e is effective only if pg_ctl is invoked as a 
Windows

service.  So it is written at register mode.  That is, -e specifies the
event source used by the Windows service which is registered by pg_ctl
register.


Oh, right. I see what you mean now. That goes for all parameters
though, including -D, and we don't specify those as register mode
only, so I still think it's wrong to place it there. It is now grouped
with all other parameters that we specifically *don't* write to the
commandline of the service.


Sorry, but I'm probably not understanding your comment.  This may be due to 
my English capability.  -e is effective only on Windows, so it is placed in 
section Options for Windows.  And I could not find a section named Common 
options.  -e is currently meangful only with register mode, so it is placed 
at register mode in Synopsis section.  For example, -D is not attached to 
kill mode.


Do you suggest that -e should be attached to all modes in Synopsis section, 
or -e should be placed in the section Options instead of Options for 
Windows?



Regards
MauMau


pg_ctl_eventsrc_v11.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-16 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com
So as a conclusion, the left over items to be handled for patch are:

1. Remove the new usage related to use of same event source name
for registration from pgevent.
2. Document the information to prevent loss of messages in some
scenarios as explained above.


I noticed the continued discussion after I had prepared and submitted the 
revised patch.  OK, how about the patch in the previous mail, Magnus-san?


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-15 Thread Magnus Hagander
On Sat, May 10, 2014 at 4:10 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 I think it's bit late for this patch for 9.4, you might want to move it to
 next CF.


 Thanks, I've moved it.  It's a regret that this very small patch wasn't put
 in 9.4.

i took a look at the latest version of this patch, since it's marked
as ready for committer. A few comments:

Is there a reason for there still being changes in guc.c, pgevent.c
etc? Shouldn't it all be confined to pg_ctl now? That's my
understanding from the thread that that's the only part we care about.

More importantly, isn't it wrong to claim it will only be used for
register and unregister? If we get an early failure in start, for
example, there are numerous codepaths that will end up calling
write_stderr(), which will use the eventlog when running as a service.
Shouldn't the -e parameter be moved under common options?

I also think we should have the documentation specifically note that
regular postgres output still goes to whatever the backend is
configured to do. (and of course, any failure within the backend
*before* we have parsed the config file for example will still go to
the default source, and not the pg_ctl source - so we need to make it
really clear that this is *only* for output from pg_ctl).


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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-15 Thread MauMau

From: Magnus Hagander mag...@hagander.net

Is there a reason for there still being changes in guc.c, pgevent.c
etc? Shouldn't it all be confined to pg_ctl now? That's my
understanding from the thread that that's the only part we care about.


Yes, strictly speaking, those are useful for the original proposal. 
However, they are still useful as refactoring, since the current code has 
the default event source PostgreSQL in many places.  Defining the default 
name only at one location would make it easier for vendors like EnterpriseDB 
to change the default name for their products.  So I hope this will also be 
included if you don't want to reject it by all means.




More importantly, isn't it wrong to claim it will only be used for
register and unregister? If we get an early failure in start, for
example, there are numerous codepaths that will end up calling
write_stderr(), which will use the eventlog when running as a service.
Shouldn't the -e parameter be moved under common options?


Yes, you are right.  -e is effective only if pg_ctl is invoked as a Windows 
service.  So it is written at register mode.  That is, -e specifies the 
event source used by the Windows service which is registered by pg_ctl 
register.




I also think we should have the documentation specifically note that
regular postgres output still goes to whatever the backend is
configured to do. (and of course, any failure within the backend
*before* we have parsed the config file for example will still go to
the default source, and not the pg_ctl source - so we need to make it
really clear that this is *only* for output from pg_ctl).


I see.  I added this clarification to the description of -e.  I would 
appreciate it if you could correct my poor English when committing, if it 
needs improvement.


Regards
MauMau


pg_ctl_eventsrc_v10.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-15 Thread Magnus Hagander
On Tue, Jul 15, 2014 at 4:01 PM, MauMau maumau...@gmail.com wrote:
 From: Magnus Hagander mag...@hagander.net

 Is there a reason for there still being changes in guc.c, pgevent.c
 etc? Shouldn't it all be confined to pg_ctl now? That's my
 understanding from the thread that that's the only part we care about.


 Yes, strictly speaking, those are useful for the original proposal. However,
 they are still useful as refactoring, since the current code has the default
 event source PostgreSQL in many places.  Defining the default name only at
 one location would make it easier for vendors like EnterpriseDB to change
 the default name for their products.  So I hope this will also be included
 if you don't want to reject it by all means.

Well, it does in a couple of places. I'm nto sure it's that important
(as nobody has AFAIK ever requested that change from for example EDB),
but it's not a bad thing. However, with a hardcoded service name, I
think the changes to pg_event.c are probably both not necessary and
actually bad - as you'll now start getting errors in more harmless
scenarios.


 More importantly, isn't it wrong to claim it will only be used for
 register and unregister? If we get an early failure in start, for
 example, there are numerous codepaths that will end up calling
 write_stderr(), which will use the eventlog when running as a service.
 Shouldn't the -e parameter be moved under common options?


 Yes, you are right.  -e is effective only if pg_ctl is invoked as a Windows
 service.  So it is written at register mode.  That is, -e specifies the
 event source used by the Windows service which is registered by pg_ctl
 register.

Oh, right. I see what you mean now. That goes for all parameters
though, including -D, and we don't specify those as register mode
only, so I still think it's wrong to place it there. It is now grouped
with all other parameters that we specifically *don't* write to the
commandline of the service.


 I also think we should have the documentation specifically note that
 regular postgres output still goes to whatever the backend is
 configured to do. (and of course, any failure within the backend
 *before* we have parsed the config file for example will still go to
 the default source, and not the pg_ctl source - so we need to make it
 really clear that this is *only* for output from pg_ctl).


 I see.  I added this clarification to the description of -e.  I would
 appreciate it if you could correct my poor English when committing, if it
 needs improvement.

Sure, no problem.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-15 Thread MauMau

From: Magnus Hagander mag...@hagander.net

Well, it does in a couple of places. I'm nto sure it's that important
(as nobody has AFAIK ever requested that change from for example EDB),
but it's not a bad thing. However, with a hardcoded service name, I
think the changes to pg_event.c are probably both not necessary and
actually bad - as you'll now start getting errors in more harmless
scenarios.


Thank you.  OK, in fact, all I want in pgevent.c is this:

! char  event_source[256] = PostgreSQL;
...
! char  event_source[256] = DEFAULT_EVENT_SOURCE;

But what kind of errors do we get with other changes in pgevent.c?  I made 
these changes according to Amit-san's notice (please look at his comment 
upthread), and I think he is right.




Oh, right. I see what you mean now. That goes for all parameters
though, including -D, and we don't specify those as register mode
only, so I still think it's wrong to place it there. It is now grouped
with all other parameters that we specifically *don't* write to the
commandline of the service.


OK, let me reconsider this tomorrow.  It's already after midnight here in 
Japan, and I have to go to bed so that I can wake up tomorrow.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-07-15 Thread Amit Kapila
On Tue, Jul 15, 2014 at 8:57 PM, MauMau maumau...@gmail.com wrote:

 From: Magnus Hagander mag...@hagander.net

 Well, it does in a couple of places. I'm nto sure it's that important
 (as nobody has AFAIK ever requested that change from for example EDB),
 but it's not a bad thing.

I think this is nothing specific to any vendor rather it's good to have
a define when it is used at multiple places.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-05-10 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

I think it's bit late for this patch for 9.4, you might want to move it to
next CF.


Thanks, I've moved it.  It's a regret that this very small patch wasn't put 
in 9.4.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-05-09 Thread Amit Kapila
On Thu, May 8, 2014 at 4:47 PM, MauMau maumau...@gmail.com wrote:
 I rebased the patch to HEAD and removed the compilation error on Linux.  I
 made event_source variable on all platforms like register_servicename,
 although they are not necessary on non-Windows platforms.

I have verified that current patch is fine and I have marked it as
Ready For Committer.

I think it's bit late for this patch for 9.4, you might want to move it to
next CF.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-05-08 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com
Only one minor suggestion:
+Name of the event source for pg_ctl to use for event log.  The
+default is PostgreSQL.


From this description, it is not clear when the event log will be used

in pg_ctl.  For example, if user uses -e option with register, then he
might expect any failure during register command will be logged into
eventlog, but that is not true.  So I think we should improve the docs
to reflect the usage of -e.

On Linux, I am getting below build failure for pg_ctl

pg_ctl.c: In function ‘main’:
pg_ctl.c:2173: error: ‘event_source’ undeclared (first use in this 
function)

pg_ctl.c:2173: error: (Each undeclared identifier is reported only once
pg_ctl.c:2173: error: for each function it appears in.)
make[3]: *** [pg_ctl.o] Error 1
make[2]: *** [all-pg_ctl-recurse] Error 2
make[1]: *** [all-bin-recurse] Error 2
make: *** [all-src-recurse] Error 2


Thank you for reviewing and testing.  I changed the doc, but I'd appreciate 
it if you could improve my poor English and update the CommitFest if it can 
be better.


I rebased the patch to HEAD and removed the compilation error on Linux.  I 
made event_source variable on all platforms like register_servicename, 
although they are not necessary on non-Windows platforms.


Regards
MauMau


pg_ctl_eventsrc_v9.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-05-03 Thread Amit Kapila
On Fri, Apr 18, 2014 at 5:21 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Currently -e option is accepted with all the options that can be provided
 in pg_ctl.  Shouldn't we accept it only with options related to service,
 because that is only when it will be used.  Basically write_stderr() will
 write to event log only incase of service.


 Thank you for your kind and patient review.  I hope this will bear fruit.

 I don't find it so strange that -e is accepted by all operation modes of
 pg_ctl --- pg_ctl seems to handle switches that way.  For example, -c is
 only relevant to start and restart, but it is accepted by all modes.  -D is
 not relevant to pg_ctl kill, but it is not rejected.  Plus, I prepared for
 the future possibility that other modes of pg_ctl will use event log.

Okay, as there is no check for not-required switches with other options,
we can ignore this as well.

I have verified the patch and it works fine on Windows, as per my
verification event-source in pg_ctl will be only used once the user has
registered the service and then when it tries to preform start/stop on
service.  Although current usecase seems to be quite narrow for a new
switch, however in future we can extend it to use for other messages
in pg_ctl as well.

Only one minor suggestion:
+Name of the event source for pg_ctl to use for event log.  The
+default is PostgreSQL.

From this description, it is not clear when the event log will be used
in pg_ctl.  For example, if user uses -e option with register, then he
might expect any failure during register command will be logged into
eventlog, but that is not true.  So I think we should improve the docs
to reflect the usage of -e.

On Linux, I am getting below build failure for pg_ctl

pg_ctl.c: In function ‘main’:
pg_ctl.c:2173: error: ‘event_source’ undeclared (first use in this function)
pg_ctl.c:2173: error: (Each undeclared identifier is reported only once
pg_ctl.c:2173: error: for each function it appears in.)
make[3]: *** [pg_ctl.o] Error 1
make[2]: *** [all-pg_ctl-recurse] Error 2
make[1]: *** [all-bin-recurse] Error 2
make: *** [all-src-recurse] Error 2


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-18 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Currently -e option is accepted with all the options that can be provided
in pg_ctl.  Shouldn't we accept it only with options related to service,
because that is only when it will be used.  Basically write_stderr() will
write to event log only incase of service.


Thank you for your kind and patient review.  I hope this will bear fruit.

I don't find it so strange that -e is accepted by all operation modes of 
pg_ctl --- pg_ctl seems to handle switches that way.  For example, -c is 
only relevant to start and restart, but it is accepted by all modes.  -D is 
not relevant to pg_ctl kill, but it is not rejected.  Plus, I prepared for 
the future possibility that other modes of pg_ctl will use event log.



Another minor point is you have forgotten to remove below declaration:
+ static void get_config_value(const char *name, char *buf, int buf_size);


Oh, removed.


Sorry for delayed response and I am not sure that I will be able to
complete the review of patch in next few days as I will be on vacation.


OK, I'm waiting for you.  Please have a nice vacation.

Regards
MauMau


pg_ctl_eventsrc_v8.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-17 Thread Amit Kapila
On Sat, Apr 12, 2014 at 1:21 PM, MauMau maumau...@gmail.com wrote:
 Hello, Amit san, Tom san,

 I'm sorry for my late response.  I've just caught up with the discussion.
 I'm almost convinced.

 Please find attached the revised patch.  I'd like to follow the idea of
 adding a switch to pg_ctl.  The newly added -e event_source sets the
 event source name for pg_ctl to use.  When -e is used with pg_ctl register,
 it will be added to the command line for Windows service (pg_ctl
 runservice).

Currently -e option is accepted with all the options that can be provided
in pg_ctl.  Shouldn't we accept it only with options related to service,
because that is only when it will be used.  Basically write_stderr() will
write to event log only incase of service.

Another minor point is you have forgotten to remove below declaration:
+ static void get_config_value(const char *name, char *buf, int buf_size);

Sorry for delayed response and I am not sure that I will be able to
complete the review of patch in next few days as I will be on vacation.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-12 Thread MauMau

Hello, Amit san, Tom san,

I'm sorry for my late response.  I've just caught up with the discussion. 
I'm almost convinced.


Please find attached the revised patch.  I'd like to follow the idea of 
adding a switch to pg_ctl.  The newly added -e event_source sets the 
event source name for pg_ctl to use.  When -e is used with pg_ctl register, 
it will be added to the command line for Windows service (pg_ctl 
runservice).


Regards
MauMau



pg_ctl_eventsrc_v7.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-09 Thread Amit Kapila
On Mon, Apr 7, 2014 at 7:10 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Sat, Apr 5, 2014 at 8:24 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 AFAICS, pg_ctl already reports to stderr if stderr is a tty.  This whole
 issue only comes up when pg_ctl itself is running as a service (which
 I guess primarily means starting/stopping the postgresql service?).
 So that puts extra weight on trying to be sure that error messages go
 to a predictable place; the user's going to be hard-pressed to debug
 background failures without that.

Agreed.  I think if this needs to fixed, then it will be better to do
as per your suggestion of adding a new switch.  So I have marked this
patch as Returned with feedback.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-07 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Sat, Apr 5, 2014 at 8:24 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 How's that going to work during pg_ctl stop?  There's no -o switch
 provided.

 As there's no -o switch, so there won't be problem of getting wrong event
 source name from server due to command line options which you mentioned
 upthread or is there something which I am missing about it?

AFAICS, the sole argument for trying to do things this way is to log to
the same event_source the running postmaster is using.  But -C will not
provide that; it will only tell you what the postmaster *would* use if
it were freshly started right now.  If -o had been used at postmaster
start time, an inquiry done by pg_ctl stop (or reload, etc) won't match.
Another, possibly more plausible, failure mode is if the entry in
postgresql.conf has been edited since the running postmaster looked at it.

Admittedly, all of these cases are kind of unusual.  But that's all the
more reason, IMO, to be wary of sending our error messages to an
unexpected place in such cases.  Somebody who's trying to debug a
configuration problem doesn't need the added complication of trying to
figure out where pg_ctl's error messages might have gone.

 You are right that with the current patch approach, we will miss many
 opportunities for failures and the way suggested by you below (new switch)
 is more appropriate to fix this issue. Another thought that occurred to me
 is why not change the failures which are before set of appropriate
 event_source to report on console. The main reason for using event log
 to report error's in pg_ctl is because it can be used for service
 (register/unregister/..) in Windows and all the work we do before setting
 event_source is not related to it's usage as a service.

AFAICS, pg_ctl already reports to stderr if stderr is a tty.  This whole
issue only comes up when pg_ctl itself is running as a service (which
I guess primarily means starting/stopping the postgresql service?).
So that puts extra weight on trying to be sure that error messages go
to a predictable place; the user's going to be hard-pressed to debug
background failures without that.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-07 Thread Robert Haas
On Sat, Apr 5, 2014 at 10:54 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 So basically, I think having pg_ctl try to do what this patch proposes
 is a bad idea.

I'm not a Windows person either, but I tend to agree.  I can't think
that this is going to be very robust ... and if it's not going to be
robust, what's the point?

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-06 Thread Amit Kapila
On Sat, Apr 5, 2014 at 8:24 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 Are you concerned about the case when user passes event_source name
 in command line at the time of start:
 pg_ctl start -o -c event_source=PG9.4 -D data_dir

 Right.

 If my understanding is right about your concern, then I think it will 
 consider
 the above case even when passed in command line. Example
 postgres.exe -C event_source -c event_source=PG9.4 -D data_dir
 PG9.4

 How's that going to work during pg_ctl stop?  There's no -o switch
 provided.

As there's no -o switch, so there won't be problem of getting wrong event
source name from server due to command line options which you mentioned
upthread or is there something which I am missing about it?

 It's conceivable that you could reverse-engineer it by looking at
 postmaster.opts as well as what -C mode has to say, and that'd likely work
 for the parameters pg_ctl cares about; but my goodness that's ugly and
 fragile.  You'd basically be reimplementing a lot of GUC logic in pg_ctl.

 In any case, the real problem is that even if you trust the -C result to
 be right, by the time you've got this information there have already been
 a whole lot of opportunities for failures.  It doesn't seem to me that
 having pg_ctl switch its error reporting target halfway through is really
 such a great idea.

You are right that with the current patch approach, we will miss many
opportunities for failures and the way suggested by you below (new switch)
is more appropriate to fix this issue. Another thought that occurred to me
is why not change the failures which are before set of appropriate
event_source to report on console. The main reason for using event log
to report error's in pg_ctl is because it can be used for service
(register/unregister/..) in Windows and all the work we do before setting
event_source is not related to it's usage as a service.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-05 Thread Amit Kapila
On Sat, Apr 5, 2014 at 4:58 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 MauMau maumau...@gmail.com writes:
 [ pg_ctl_eventsrc_v6.patch ]

 I looked at this patch a bit.  As a non-Windows person I have no intention
 of being the committer, since I can't test the Windows-specific changes.
 However, I do want to object to the business about having pg_ctl use
 postgres -C to try to determine the event source name to use.  The code
 that you repurposed was okay for its original use, namely finding out
 where a config directory would point the data directory to, but I don't
 think it's up to snuff for identifying the value of event_source that a
 running server is using.  The actual value might have been set from a
 command line option for instance.

Are you concerned about the case when user passes event_source name
in command line at the time of start:
pg_ctl start -o -c event_source=PG9.4 -D data_dir

If my understanding is right about your concern, then I think it will consider
the above case even when passed in command line. Example
postgres.exe -C event_source -c event_source=PG9.4 -D data_dir
PG9.4

 Moreover, the whole thing seems rather
 circular since what pg_ctl wants the event source name for is to report
 errors.  What if it fails to get the event source name, or needs to
 report an error before the (rather late) point at which it even tries
 to get it?

In that case, it will use Default Event Source name PostgreSQL which
is same what server also does in case it gets error before processing
of event source config. For Example if we get any error in check_root()
function, it will use Default Event Source name.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-05 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 Are you concerned about the case when user passes event_source name
 in command line at the time of start:
 pg_ctl start -o -c event_source=PG9.4 -D data_dir

Right.

 If my understanding is right about your concern, then I think it will consider
 the above case even when passed in command line. Example
 postgres.exe -C event_source -c event_source=PG9.4 -D data_dir
 PG9.4

How's that going to work during pg_ctl stop?  There's no -o switch
provided.

It's conceivable that you could reverse-engineer it by looking at
postmaster.opts as well as what -C mode has to say, and that'd likely work
for the parameters pg_ctl cares about; but my goodness that's ugly and
fragile.  You'd basically be reimplementing a lot of GUC logic in pg_ctl.

In any case, the real problem is that even if you trust the -C result to
be right, by the time you've got this information there have already been
a whole lot of opportunities for failures.  It doesn't seem to me that
having pg_ctl switch its error reporting target halfway through is really
such a great idea.

 In that case, it will use Default Event Source name PostgreSQL which
 is same what server also does in case it gets error before processing
 of event source config.

That analogy doesn't hold because (1) a server spends most of its time
already up, so only a tiny fraction of its error traffic might go to the
default event source, and (2) actually, pretty much *none* of a server's
log traffic will go to the default event source if something else has been
configured, because the default value of log_destination isn't eventlog.
There might be a small window where GUC has assigned log_destination but
not yet event_source from postgresql.conf, but it's just about negligible.

In contrast, a large fraction of pg_ctl's possible error traffic is
concerned with early failures like can't-find-the-postgres-executable
or can't-find-the-data-directory, either of which would foreclose any
possibility of selecting an event source that matches the server.

So basically, I think having pg_ctl try to do what this patch proposes
is a bad idea.  If there's anyone who actually cares about where pg_ctl
logs to on Windows, what would make sense is to add a pg_ctl switch to
specify what event_source name to use.  That's still not perfect of
course, but at least only command-line-parsing errors would come out
before the setting could be adopted.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-04-04 Thread Tom Lane
MauMau maumau...@gmail.com writes:
 [ pg_ctl_eventsrc_v6.patch ]

I looked at this patch a bit.  As a non-Windows person I have no intention
of being the committer, since I can't test the Windows-specific changes.
However, I do want to object to the business about having pg_ctl use
postgres -C to try to determine the event source name to use.  The code
that you repurposed was okay for its original use, namely finding out
where a config directory would point the data directory to, but I don't
think it's up to snuff for identifying the value of event_source that a
running server is using.  The actual value might have been set from a
command line option for instance.  Moreover, the whole thing seems rather
circular since what pg_ctl wants the event source name for is to report
errors.  What if it fails to get the event source name, or needs to
report an error before the (rather late) point at which it even tries
to get it?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-21 Thread MauMau

From: Alvaro Herrera alvhe...@2ndquadrant.com

MauMau escribió:


The raw link only gave the mail in text format.  I hoped to import
the mail into Windows Mail on Windows Vista, but I couldn't.


You might need to run a conversion process by which you transform the
raw file (in mbox format) into EML format or whatever it is that Windows
Mail uses.  I vaguely recall there are tools for this.


Thanks.  I could open the file without any conversion as follows:

1. Click the raw link on the Web browser (I'm using Internet Explorer).

2. The Web browser displays the mail file in text format.  Save the file as 
a text file (e.g. mail.txt).


3. Just change the extension from .txt to .eml (e.g. mail.eml).

4. Double-click the .eml file on the Windows Explorer.  Windows Mail opens 
and displayes the mail.  I can reply to it.


Regards
MauMau




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-12 Thread MauMau

From: Alvaro Herrera alvhe...@2ndquadrant.com

MauMau escribió:

Do you know how I can reply to an email which was deleted locally?
I thought I could download an old mail by clicking raw link and
import it to the mailer.  However, it requires username/password
input, and it seems to be different from the one for editing
CommitFest.  I couldn't find how to authenticate myself.


The box that asks for password tells you what the user/password is.  I
think it's something like archives/archives or similar.  The password is
there only to keep spammers out, not to have any real auth.


Thank you, the user/password was certainly displayed in the box --  
archives/antispam.  The raw link only gave the mail in text format.  I 
hoped to import the mail into Windows Mail on Windows Vista, but I couldn't.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-12 Thread Alvaro Herrera
MauMau escribió:

 The raw link only gave the mail in text format.  I hoped to import
 the mail into Windows Mail on Windows Vista, but I couldn't.

You might need to run a conversion process by which you transform the
raw file (in mbox format) into EML format or whatever it is that Windows
Mail uses.  I vaguely recall there are tools for this.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-11 Thread Alvaro Herrera
MauMau escribió:
 Hi, Amit san,
 
 I'm replying to your previous email.  I wanted to reply to your
 latest mail below, but I removed it from my mailer by mistake.
 
 http://www.postgresql.org/message-id/CAA4eK1LAg6ndZdWLb5e=Ep5DzcE8KZU=JbmO+tFwySYHm2ja=q...@mail.gmail.com
 
 Do you know how I can reply to an email which was deleted locally?
 I thought I could download an old mail by clicking raw link and
 import it to the mailer.  However, it requires username/password
 input, and it seems to be different from the one for editing
 CommitFest.  I couldn't find how to authenticate myself.

The box that asks for password tells you what the user/password is.  I
think it's something like archives/archives or similar.  The password is
there only to keep spammers out, not to have any real auth.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-10 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

If I understand correctly that objection was on changing Default Event
Source name, and the patch now doesn't contain that change, it's
just a bug fix for letting pg_ctl know the non-default event source
set by user.

Please clarify if I misunderstood something, else this should be changed
to Ready For Committer.


Tom/Andres, please let me know if you have objection for this patch, 
because

as per my understanding all the objectionable part of patch is removed
from final
patch and it's a defect fix to make pg_ctl aware of Event Source name set 
in

postgresql.conf.

If there is no objection, I will again change it to Ready For Committer.


Hi, Amit-san, I really appreciate your cooperation.  Yes, I removed the 
default value change that caused objection, so the patch can be marked ready 
for committer.  I understand the patch was marked needs for review by 
misunderstanding Tom-san's opinion.


I remember that I read silence means no objection, or implicit agreement 
somewhere in the community site or ML.  So I think it would be no problem to 
set the status to ready for committer again.


Regards
MauMau




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-10 Thread Amit Kapila
On Mon, Mar 10, 2014 at 2:39 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 If I understand correctly that objection was on changing Default Event
 Source name, and the patch now doesn't contain that change, it's
 just a bug fix for letting pg_ctl know the non-default event source
 set by user.

 Please clarify if I misunderstood something, else this should be changed
 to Ready For Committer.


 Tom/Andres, please let me know if you have objection for this patch,
 because
 as per my understanding all the objectionable part of patch is removed
 from final
 patch and it's a defect fix to make pg_ctl aware of Event Source name set
 in
 postgresql.conf.

 If there is no objection, I will again change it to Ready For Committer.


 Hi, Amit-san, I really appreciate your cooperation.

Thanks.

 Yes, I removed the
 default value change that caused objection, so the patch can be marked ready
 for committer.  I understand the patch was marked needs for review by
 misunderstanding Tom-san's opinion.

 I remember that I read silence means no objection, or implicit agreement
 somewhere in the community site or ML.  So I think it would be no problem to
 set the status to ready for committer again.

Okay, Done.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-03-09 Thread Amit Kapila
On Mon, Feb 17, 2014 at 9:17 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Sat, Feb 1, 2014 at 12:31 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 I think it's just a very minor coding style thing, so I am marking this 
 patch as
 Ready For Committer.

 I could see that this patch has been marked as Needs Review in CF app.
 suggesting that it should be rejected based on Tom's rejection in below mail:
 http://www.postgresql.org/message-id/3315.1390836...@sss.pgh.pa.us

 If I understand correctly that objection was on changing Default Event
 Source name, and the patch now doesn't contain that change, it's
 just a bug fix for letting pg_ctl know the non-default event source
 set by user.

 Please clarify if I misunderstood something, else this should be changed
 to Ready For Committer.

Tom/Andres, please let me know if you have objection for this patch, because
as per my understanding all the objectionable part of patch is removed
from final
patch and it's a defect fix to make pg_ctl aware of Event Source name set in
postgresql.conf.

If there is no objection, I will again change it to Ready For Committer.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-02-16 Thread Amit Kapila
On Sat, Feb 1, 2014 at 12:31 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 I think it's just a very minor coding style thing, so I am marking this patch 
 as
 Ready For Committer.

I could see that this patch has been marked as Needs Review in CF app.
suggesting that it should be rejected based on Tom's rejection in below mail:
http://www.postgresql.org/message-id/3315.1390836...@sss.pgh.pa.us

If I understand correctly that objection was on changing Default Event
Source name, and the patch now doesn't contain that change, it's
just a bug fix for letting pg_ctl know the non-default event source
set by user.

Please clarify if I misunderstood something, else this should be changed
to Ready For Committer.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-31 Thread MauMau

Hi, Amit san,

I'm replying to your previous email.  I wanted to reply to your latest mail 
below, but I removed it from my mailer by mistake.


http://www.postgresql.org/message-id/CAA4eK1LAg6ndZdWLb5e=Ep5DzcE8KZU=JbmO+tFwySYHm2ja=q...@mail.gmail.com

Do you know how I can reply to an email which was deleted locally?  I 
thought I could download an old mail by clicking raw link and import it to 
the mailer.  However, it requires username/password input, and it seems to 
be different from the one for editing CommitFest.  I couldn't find how to 
authenticate myself.


Anyway, the revised patch is attached.

From: Amit Kapila amit.kapil...@gmail.com

It gives the proper message, but even after error, the second message
box it shows DLLInstall ... succeeded. I think the reason is that caller
of function DllRegisterServer() doesn't check the return value.


I see.  Corrected by checking the return value of DllRegisterServer().



 + char message[1024];


why you have kept message as a global buffer, can't we just declare 
locally

inside the function?


I made it a local variable.  At first, I thought we might use it in other 
functions in the future.



Okay, I think we can leave it and also remove it from other parts of 
patch.
Although I found it is the right way, but Tom is not convinced with the 
idea,

so lets keep the Default event source name handling as it is.


OK, I changed the value of DEFAULT_EVENT_SOURCE to PostgreSQL.



As suggested by Tom, please update documentation.
 Possibly there's room for a documentation patch reminding users to
 make sure that event_source is set appropriately before they turn
 on eventlog.
I think right place to update this information is where we are explaining
about setting of event log i.e at below link or may be if you find some 
other

better place:
http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-LOG-DESTINATION


Please let us make this a separate patch.  I agree with you about the place 
in the manual.


Regards
MauMau


pg_ctl_eventsrc_v6.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-31 Thread Amit Kapila
On Fri, Jan 31, 2014 at 8:20 PM, MauMau maumau...@gmail.com wrote:
 Hi, Amit san,

 I'm replying to your previous email.  I wanted to reply to your latest mail
 below, but I removed it from my mailer by mistake.

 http://www.postgresql.org/message-id/CAA4eK1LAg6ndZdWLb5e=Ep5DzcE8KZU=JbmO+tFwySYHm2ja=q...@mail.gmail.com

 Do you know how I can reply to an email which was deleted locally?  I
 thought I could download an old mail by clicking raw link and import it to
 the mailer.  However, it requires username/password input, and it seems to
 be different from the one for editing CommitFest.  I couldn't find how to
 authenticate myself.

Not sure, I have not done it before.

 Anyway, the revised patch is attached.

 From: Amit Kapila amit.kapil...@gmail.com


 As suggested by Tom, please update documentation.
  Possibly there's room for a documentation patch reminding users to
  make sure that event_source is set appropriately before they turn
  on eventlog.
 I think right place to update this information is where we are explaining
 about setting of event log i.e at below link or may be if you find some
 other
 better place:

 http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-LOG-DESTINATION


 Please let us make this a separate patch.  I agree with you about the place
 in the manual.
Okay, no problem.

As per my review your patch is fine now except for one minor indentation issue.

if (RegCreateKeyEx(HKEY_LOCAL_MACHINE, key_name, 0, NULL, 0, KEY_WRITE,
NULL, key, data))

Here it is better to start the parameters in second line from where
the params in
first line, make then inline.

I think it's just a very minor coding style thing, so I am marking this patch as
Ready For Committer.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-28 Thread Amit Kapila
On Fri, Jan 24, 2014 at 4:38 PM, MauMau maumau...@gmail.com wrote:


 How about below message:

 event source event_source_name is already registered.

 OK, I added several lines for this.  Please check the attached patch.

It gives the proper message, but even after error, the second message
box it shows DLLInstall ... succeeded. I think the reason is that caller
of function DllRegisterServer() doesn't check the return value.

 + char message[1024];

why you have kept message as a global buffer, can't we just declare locally
inside the function?

 What I had in mind was to change it during initdb, we are already doing it
 for some other parameter (unix_socket_directories), please refer below
 code in initdb.c
 Yes, It seems we can do this.  However, could you forgive me for leaving this 
 untouched?  I'm afraid postgresql.conf.sample's issue is causing
 unnecessary war among people here.  That doesn't affect the point of this 
 patch --- make pg_ctl use the event_source setting.  Anyway, not all
 parameters in postgresql.conf cannot be used just by uncommenting them. 
 That's another issue.

Okay, I think we can leave it and also remove it from other parts of patch.
Although I found it is the right way, but Tom is not convinced with the idea,
so lets keep the Default event source name handling as it is.

As suggested by Tom, please update documentation.
 Possibly there's room for a documentation patch reminding users to
 make sure that event_source is set appropriately before they turn
 on eventlog.
I think right place to update this information is where we are explaining
about setting of event log i.e at below link or may be if you find some other
better place:
http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-LOG-DESTINATION


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-27 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Fri, Jan 24, 2014 at 9:10 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Fri, Jan 24, 2014 at 2:38 AM, Robert Haas robertmh...@gmail.com wrote:
 I mean, if
 there's a GUC that controls the event source name, then it can be
 changed between restarts, regardless of what you call it.

 Yes, but not default values (when user don't provide any value
 for event_soource). Here the question is about default value of
 event_source.

 To proceed with the review of this patch, I need to know about
 whether appending version number or any other constant togli

 Default Event Source name is acceptable or not, else for now
 we can remove this part of code from patch and handle non-default
 case where the change will be that pg_ctl will enquire non-default
 event_source value from server.

 Could you please let me know your views about same?

Unless I'm missing something, this entire thread is a tempest in a teapot,
because the default event_source value does not matter, because *by
default we don't log to eventlog*.  The presumption is that if the user
turns on logging to eventlog, it's his responsibility to first make sure
that event_source is set to something appropriate.  And who's to say that
plain PostgreSQL isn't what he wanted, anyway?  Even if he's got
multiple servers on one machine, maybe directing all their logs to the
same place is okay by him.

Also, those who don't run multiple servers are probably not going to
thank us for moving their logs around unnecessarily.

In short, I think we should just reject this idea as introducing more
problems than it solves, and not fully solving even the problem it
purports to solve.

Possibly there's room for a documentation patch reminding users to
make sure that event_source is set appropriately before they turn
on eventlog.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-27 Thread Amit Kapila
On Fri, Jan 24, 2014 at 9:10 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Fri, Jan 24, 2014 at 2:38 AM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jan 23, 2014 at 9:23 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Jan 23, 2014 at 10:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:

 I think what we might want to do is redefine the server's behavior
 as creating an event named after the concatenation of event_source
 and port number, or maybe even get rid of event_source entirely and
 just say it's PostgreSQL followed by the port number.

To accomplish this behaviour, each time server starts and stops,
we need to register and unregister event log using mechanism
described at below link to ensure that there is no mismatch between
what server uses and what OS knows.
http://www.postgresql.org/docs/devel/static/event-log-registration.html

 Why wouldn't that be necessary with your approach, too?

Because in my approach we are using compile time constant
 + #define DEFAULT_EVENT_SOURCE
PostgreSQL  PG_MAJORVERSION

 I mean, if
 there's a GUC that controls the event source name, then it can be
 changed between restarts, regardless of what you call it.

 Yes, but not default values (when user don't provide any value
 for event_soource). Here the question is about default value of
 event_source.

To proceed with the review of this patch, I need to know about
whether appending version number or any other constant to
Default Event Source name is acceptable or not, else for now
we can remove this part of code from patch and handle non-default
case where the change will be that pg_ctl will enquire non-default
event_source value from server.

Could you please let me know your views about same?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-27 Thread Amit Kapila
On Mon, Jan 27, 2014 at 9:01 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Fri, Jan 24, 2014 at 9:10 AM, Amit Kapila amit.kapil...@gmail.com wrote:

 To proceed with the review of this patch, I need to know about
 whether appending version number or any other constant togli

 Default Event Source name is acceptable or not, else for now
 we can remove this part of code from patch and handle non-default
 case where the change will be that pg_ctl will enquire non-default
 event_source value from server.

 Could you please let me know your views about same?

 Unless I'm missing something, this entire thread is a tempest in a teapot,
 because the default event_source value does not matter, because *by
 default we don't log to eventlog*.  The presumption is that if the user
 turns on logging to eventlog, it's his responsibility to first make sure
 that event_source is set to something appropriate.  And who's to say that
 plain PostgreSQL isn't what he wanted, anyway?  Even if he's got
 multiple servers on one machine, maybe directing all their logs to the
 same place is okay by him.

I think it's matter of user preference, how exactly he wants the setup
and as currently we don't have any strong reason to change default, so
lets keep it intact.

 Also, those who don't run multiple servers are probably not going to
 thank us for moving their logs around unnecessarily.

 In short, I think we should just reject this idea as introducing more
 problems than it solves, and not fully solving even the problem it
 purports to solve.

 Possibly there's room for a documentation patch reminding users to
 make sure that event_source is set appropriately before they turn
 on eventlog.

 Okay, but in that case also right now pg_ctl doesn't know the value
 of event source, so I think thats a clear bug and we should go ahead
 and fix it.

 As you said, I think we can improve documentation in this regard so
 that user will be able to setup event log without any such problems.

 As part of this patch we can fix the issue (make pg_ctl aware for event
 source name) and improve documentation.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-24 Thread MauMau

From: MauMau maumau...@gmail.com

OK, I added several lines for this.  Please check the attached patch.


I'm sorry, I attached the old patch as v5 in my previous mail.  Attached on 
this mail is the correct one.


I'll update the CommitFest entry soon.

Regards
MauMau


pg_ctl_eventsrc_v5.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Amit Kapila
On Thu, Jan 23, 2014 at 10:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 So?  Anything which can know the value of a GUC parameter can certainly
 know the selected port number.

 1. In case of registration of event source, either user has to pass the name
 or it uses hard coded default value, so if we use version number along 
 with
 'PostgreSQL', it can be consistent.
 I don't see any way pgevent.c can know port number to append it to 
 default
 value, am I missing something here?


 I think what we might want to do is redefine the server's behavior
 as creating an event named after the concatenation of event_source
 and port number, or maybe even get rid of event_source entirely and
 just say it's PostgreSQL followed by the port number.

   To accomplish this behaviour, each time server starts and stops,
   we need to register and unregister event log using mechanism
   described at below link to ensure that there is no mismatch between
   what server uses and what OS knows.
   http://www.postgresql.org/docs/devel/static/event-log-registration.html

   IIUC, this will be a much larger behaviour change.
   What is the problem you are envisioning if we use version number,
   yesterday I have given some examples about other softwares which
   are registered in event log and uses version number, so I don't
   understand why it is big deal if we also use version number along with
   PostgreSQL as a default value and if user specifies any particular name
   then use the same.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Robert Haas
On Thu, Jan 23, 2014 at 9:23 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Jan 23, 2014 at 10:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 So?  Anything which can know the value of a GUC parameter can certainly
 know the selected port number.

 1. In case of registration of event source, either user has to pass the name
 or it uses hard coded default value, so if we use version number along 
 with
 'PostgreSQL', it can be consistent.
 I don't see any way pgevent.c can know port number to append it to 
 default
 value, am I missing something here?


 I think what we might want to do is redefine the server's behavior
 as creating an event named after the concatenation of event_source
 and port number, or maybe even get rid of event_source entirely and
 just say it's PostgreSQL followed by the port number.

To accomplish this behaviour, each time server starts and stops,
we need to register and unregister event log using mechanism
described at below link to ensure that there is no mismatch between
what server uses and what OS knows.
http://www.postgresql.org/docs/devel/static/event-log-registration.html

Why wouldn't that be necessary with your approach, too?  I mean, if
there's a GUC that controls the event source name, then it can be
changed between restarts, regardless of what you call it.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

How about below message:

event source event_source_name is already registered.


OK, I added several lines for this.  Please check the attached patch.



What I had in mind was to change it during initdb, we are already doing it
for some other parameter (unix_socket_directories), please refer below
code in initdb.c


Yes, It seems we can do this.  However, could you forgive me for leaving 
this untouched?  I'm afraid postgresql.conf.sample's issue is causing 
unnecessary war among people here.  That doesn't affect the point of this 
patch --- make pg_ctl use the event_source setting.  Anyway, not all 
parameters in postgresql.conf cannot be used just by uncommenting them. 
That's another issue.


I'll update the CommitFest entry soon.

Regards
MauMau


pg_ctl_eventsrc_v5.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Peter Eisentraut
On 1/23/14, 4:08 PM, Robert Haas wrote:
 Why wouldn't that be necessary with your approach, too?  I mean, if
 there's a GUC that controls the event source name, then it can be
 changed between restarts, regardless of what you call it.

I don't know if it's practical, but the logical conclusion here would be
to use an identifier that you cannot change, such as the system identifier.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Tom Lane
Peter Eisentraut pete...@gmx.net writes:
 On 1/23/14, 4:08 PM, Robert Haas wrote:
 Why wouldn't that be necessary with your approach, too?  I mean, if
 there's a GUC that controls the event source name, then it can be
 changed between restarts, regardless of what you call it.

 I don't know if it's practical, but the logical conclusion here would be
 to use an identifier that you cannot change, such as the system identifier.

That particular ID would be a horrid choice, because we don't try very
hard to ensure it's unique.  In particular, a standby server on the same
machine as the master (not an uncommon case, at least for testing
purposes) would be a guaranteed fail with that approach.

I'm still not clear on why we can't just use the port number.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Alvaro Herrera
Tom Lane escribió:
 Peter Eisentraut pete...@gmx.net writes:
  On 1/23/14, 4:08 PM, Robert Haas wrote:
  Why wouldn't that be necessary with your approach, too?  I mean, if
  there's a GUC that controls the event source name, then it can be
  changed between restarts, regardless of what you call it.
 
  I don't know if it's practical, but the logical conclusion here would be
  to use an identifier that you cannot change, such as the system identifier.
 
 That particular ID would be a horrid choice, because we don't try very
 hard to ensure it's unique.  In particular, a standby server on the same
 machine as the master (not an uncommon case, at least for testing
 purposes) would be a guaranteed fail with that approach.
 
 I'm still not clear on why we can't just use the port number.

I wonder if it would make sense to generate a unique name at some
initial point in the history of the service (perhaps at initdb time, or
at the first postmaster start) and store this name in a special,
separate file in PGDATA.  On subsequent starts we read the name from
there and always use it consistently.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Tom Lane
Alvaro Herrera alvhe...@2ndquadrant.com writes:
 Tom Lane escribió:
 That particular ID would be a horrid choice, because we don't try very
 hard to ensure it's unique.  In particular, a standby server on the same
 machine as the master (not an uncommon case, at least for testing
 purposes) would be a guaranteed fail with that approach.

 I wonder if it would make sense to generate a unique name at some
 initial point in the history of the service (perhaps at initdb time, or
 at the first postmaster start) and store this name in a special,
 separate file in PGDATA.  On subsequent starts we read the name from
 there and always use it consistently.

Meh --- that would have the same behavior as the system identifier,
ie it would propagate to slave servers without extraordinary (and
easily bypassed) measures to prevent it.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread MauMau

From: Tom Lane t...@sss.pgh.pa.us

I'm still not clear on why we can't just use the port number.


It will be possible to use port to set the default value of event_source GUC 
when starting postmaster.  But using port during event source registration 
will involve much more.
To use port, we have to tell the location of $PGDATA to regsvr32.exe. 
However, regsvr32.exe can only take an argument from /i, and we are using /i 
for event source name specification.  If we want to pass data directory, we 
have to change the usage.  Instead, we could probably have regsvr32.exe 
check PGDATA env variable and invoke postgres -C event_source, but that 
would require much more complicated code (e.g. for locating postgres.exe, 
because regsvr32.exe is in Windows directory)


Anyway, the point of my patch is to just make pg_ctl use event_source GUC 
for outputing to event log.  I want to rely on postgres -C, because pg_ctl 
already uses it for retrieving data_directory GUC.  I'd like to avoid 
further complication in code and discussion.  If you request, I can revert 
the default value of event_source and regsvr32.exe /i to PostgreSQL.  I'm 
okay with that, because syslog_ident also has the default value postgres, 
which doesn't contain the major release.


Any (not complicated) suggestions?

Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Tom Lane
MauMau maumau...@gmail.com writes:
 From: Tom Lane t...@sss.pgh.pa.us
 I'm still not clear on why we can't just use the port number.

 To use port, we have to tell the location of $PGDATA to regsvr32.exe. 

[ scratches head... ]  Exactly which of the other proposals *doesn't*
require that?  Certainly anything that involves parsing settings out
of postgresql.conf will.

A more significant problem, probably, is that even knowing $PGDATA doesn't
tell you the port number with certainty, since the postmaster might end
up taking the port number from some other source than postgresql.conf
(command line, PGPORT environment, ...).  We used to require that pg_ctl
know the port accurately, and it was a big step forward in reliability
when we got rid of that; so maybe going back to that is not such a good
idea.

I note though that pg_ctl does still need to know accurately where $PGDATA
is.  Is there any particular length limit on event source names?  Maybe
the full path to $PGDATA could be used instead of the port number.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Amit Kapila
On Fri, Jan 24, 2014 at 2:38 AM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jan 23, 2014 at 9:23 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, Jan 23, 2014 at 10:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 So?  Anything which can know the value of a GUC parameter can certainly
 know the selected port number.

 1. In case of registration of event source, either user has to pass the 
 name
 or it uses hard coded default value, so if we use version number along 
 with
 'PostgreSQL', it can be consistent.
 I don't see any way pgevent.c can know port number to append it to 
 default
 value, am I missing something here?


 I think what we might want to do is redefine the server's behavior
 as creating an event named after the concatenation of event_source
 and port number, or maybe even get rid of event_source entirely and
 just say it's PostgreSQL followed by the port number.

To accomplish this behaviour, each time server starts and stops,
we need to register and unregister event log using mechanism
described at below link to ensure that there is no mismatch between
what server uses and what OS knows.
http://www.postgresql.org/docs/devel/static/event-log-registration.html

 Why wouldn't that be necessary with your approach, too?

   Because in my approach we are using compile time constant
+ #define DEFAULT_EVENT_SOURCE
   PostgreSQL  PG_MAJORVERSION

 I mean, if
 there's a GUC that controls the event source name, then it can be
 changed between restarts, regardless of what you call it.

Yes, but not default values (when user don't provide any value
for event_soource). Here the question is about default value of
event_source.

I will try to explain by example, focus of example is for case when
user doesn't provide any value for event_source. The steps he needs
to follow are as below:

1. initdb -D data_dir
2. regsvr32 install_dir_path\lib\pgevent32.dll
3. Modify postgresql.conf to set log_destination= 'eventlog'
4. pg_ctl.exe start -D data_dir

Now for above, currently we use 'PostgreSQL' as default name
for event source both during registration (step-2) and server start
by pg_ctl (step-4). This will work fine, user will be able to see proper
messages in event log (Windows EventViewer), however if user uses
different versions on same m/c and follows above steps for both
versions, then there is a chance (incase user unistall one of version),
that EventViewer will not display PostgreSQL messages properly,
it will show something like event source not found.

To resolve this case, we thought of appending version number to
'PostgreSQL' as a default name of event source. It might not work
in all cases (for ex. 2 instances of same postgres version), but
will work in many cases where currently it doesn't work.

Now the problem for using port number is in step-2 of above case,
currently pgevent.c has no way of knowing that port number value,
let us say we teach in some way like MauMau said by passing
parameter using /i option of regsvr32, but it might become confusing
for user to use that option, because currently it is used for passing
event source name (non-default case).

If  appending some compile time constant (if you have anything other
than version number which is compile time const in your mind,
that could also work equally easy) to default name doesn't
sound to be viable fix for above problem, I think it is better to take
that out of patch and may be it can be taken up as a separate patch.



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-23 Thread Amit Kapila
On Fri, Jan 24, 2014 at 4:22 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 MauMau maumau...@gmail.com writes:
 From: Tom Lane t...@sss.pgh.pa.us
 I'm still not clear on why we can't just use the port number.

 To use port, we have to tell the location of $PGDATA to regsvr32.exe.

 [ scratches head... ]  Exactly which of the other proposals *doesn't*
 require that?

   Appending it with version number which is compile time constant.

 Certainly anything that involves parsing settings out
 of postgresql.conf will.

   We don't need to parse for default value of event source, it is only
   for case when user gives some specific name to event_source in
   postgresql.conf and it that case, we expect that user provides the
  same name during register command of event source, some thing
  like:
  regsvr32 /n /i: PostgreSQL_HEAD install_dir_path\lib\pgevent32.dll

  Here point to note is that pgevent.dll never does any parsing or lookup
  for event source name, either user provides it or we use default value.



With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-22 Thread Robert Haas
On Tue, Jan 21, 2014 at 11:20 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Wed, Jan 22, 2014 at 9:19 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Tue, Jan 21, 2014 at 6:57 PM, MauMau maumau...@gmail.com wrote:
 To follow this, we have the line as:

 #event_source = 'PostgreSQL 9.4'

 But this requires us to change this line for each major release.  That's a
 maintenance headache.

 What I had in mind was to change it during initdb, we are already doing it
 for some other parameter (unix_socket_directories),

 What happens when somebody copies their postgresql.conf from an older
 version?  That's hardly uncommon, even though it might be considered bad
 practice.  I don't think it's a good idea to try to insert a version
 identifier this way.

 But ... what's the point of including the PG version in this string
 anyhow?  If you're trying to make the string unique across different
 installations on the same machine, it's surely insufficient, and if
 that's not the point then what is?

 Well, certainly it cannot handle all different scenario's (particularly
 same version installations), but the original report for this case was
 for different versions of server. I think chances of having different
 versions of server are much more, which will be handled by this
 case.

I wonder if the port number wouldn't be a better choice.  And that
could even be done without adding a parameter.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-22 Thread Amit Kapila
On Wed, Jan 22, 2014 at 6:54 PM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 21, 2014 at 11:20 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Wed, Jan 22, 2014 at 9:19 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Tue, Jan 21, 2014 at 6:57 PM, MauMau maumau...@gmail.com wrote:
 To follow this, we have the line as:

 #event_source = 'PostgreSQL 9.4'

 But this requires us to change this line for each major release.  That's a
 maintenance headache.

 What I had in mind was to change it during initdb, we are already doing it
 for some other parameter (unix_socket_directories),

 What happens when somebody copies their postgresql.conf from an older
 version?  That's hardly uncommon, even though it might be considered bad
 practice.  I don't think it's a good idea to try to insert a version
 identifier this way.

 But ... what's the point of including the PG version in this string
 anyhow?  If you're trying to make the string unique across different
 installations on the same machine, it's surely insufficient, and if
 that's not the point then what is?

 Well, certainly it cannot handle all different scenario's (particularly
 same version installations), but the original report for this case was
 for different versions of server. I think chances of having different
 versions of server are much more, which will be handled by this
 case.

 I wonder if the port number wouldn't be a better choice.  And that
 could even be done without adding a parameter.

We need this for register of event source which might be done before
start of server.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-22 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 6:54 PM, Robert Haas robertmh...@gmail.com wrote:
 I wonder if the port number wouldn't be a better choice.  And that
 could even be done without adding a parameter.

 We need this for register of event source which might be done before
 start of server.

So?  Anything which can know the value of a GUC parameter can certainly
know the selected port number.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-22 Thread Amit Kapila
On Wed, Jan 22, 2014 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 6:54 PM, Robert Haas robertmh...@gmail.com wrote:
 I wonder if the port number wouldn't be a better choice.  And that
 could even be done without adding a parameter.

 We need this for register of event source which might be done before
 start of server.

 So?  Anything which can know the value of a GUC parameter can certainly
 know the selected port number.

1. In case of registration of event source, either user has to pass the name
or it uses hard coded default value, so if we use version number along with
'PostgreSQL', it can be consistent.
I don't see any way pgevent.c can know port number to append it to default
value, am I missing something here?
2. In pg_ctl if we use port number, then if user changes it across multiple
server restarts, then it can also create a problem, as the event source
name used will be different than what the name used during registration
of event source.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-22 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Wed, Jan 22, 2014 at 9:03 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 So?  Anything which can know the value of a GUC parameter can certainly
 know the selected port number.

 1. In case of registration of event source, either user has to pass the name
 or it uses hard coded default value, so if we use version number along 
 with
 'PostgreSQL', it can be consistent.
 I don't see any way pgevent.c can know port number to append it to default
 value, am I missing something here?

[ shrug... ]  But the same problem applies just as much to any attempt by
pg_ctl to know *any* postmaster parameter.  In particular, this entire
patch is bogus, because the value it extracts from the postgresql.conf
file does not necessarily have anything to do with the setting the live
postmaster is actually using (which might be determined by a command-line
parameter or environment variable instead).  If the technique could be
relied on, then it could be relied on just as much to extract the
postmaster's port setting.

I don't necessarily object to teaching pg_ctl to make a best-effort
estimate of a postmaster parameter in this way.  But it's complete folly
to suppose that you can get an accurate value of event_source but not
the port number.

I think what we might want to do is redefine the server's behavior
as creating an event named after the concatenation of event_source
and port number, or maybe even get rid of event_source entirely and
just say it's PostgreSQL followed by the port number.  If we did
the latter then the problem would reduce to whether pg_ctl knows
the port number, which I think we're already assuming for other
reasons.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-21 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

How about below message:

event source event_source_name is already registered.


Thanks.  I'll use this, making the initial letter the capital E like other 
messages in the same source file.  I'm going to submit the final patch and 
update the CommitFest entry tomorrow at the earliest.




Today, I reviewed the patch again and found it okay, except a small
inconsistency which is about default event source name in
postgresql.conf, all other places it's changed except in .conf file.
Do you think it makes sense to change there as well?


Oh, I missed it.  postgresql.conf.sample says:

# The commented-out settings shown in this file represent the default 
values.


To follow this, we have the line as:

#event_source = 'PostgreSQL 9.4'

But this requires us to change this line for each major release.  That's a 
maintenance headache.  Another idea is:


#event_source = 'PostgreSQL major_release'

But this is not the exact default value.

So I want to leave the line as now.  Anyway, some other lines in 
postgresql.conf.sample do not represent the default value, either,:


#data_directory = 'ConfigDir'  # use data in another directory
#max_stack_depth = 2MB   # min 100kB
#include_if_exists = 'exists.conf' # include file only if it exists
#include = 'special.conf'  # include file

Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-21 Thread Amit Kapila
On Tue, Jan 21, 2014 at 6:57 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com
 Today, I reviewed the patch again and found it okay, except a small
 inconsistency which is about default event source name in
 postgresql.conf, all other places it's changed except in .conf file.
 Do you think it makes sense to change there as well?


 Oh, I missed it.  postgresql.conf.sample says:

 # The commented-out settings shown in this file represent the default
 values.

 To follow this, we have the line as:

 #event_source = 'PostgreSQL 9.4'

 But this requires us to change this line for each major release.  That's a
 maintenance headache.

What I had in mind was to change it during initdb, we are already doing it
for some other parameter (unix_socket_directories), please refer below
code in initdb.c

#ifdef HAVE_UNIX_SOCKETS
snprintf(repltok, sizeof(repltok), #unix_socket_directories = '%s',
DEFAULT_PGSOCKET_DIR);
#else
snprintf(repltok, sizeof(repltok), #unix_socket_directories = '');
#endif
conflines = replace_token(conflines, #unix_socket_directories = '/tmp',
   repltok);

Could you once check if it is possible in above way to change
event_source?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-21 Thread Tom Lane
Amit Kapila amit.kapil...@gmail.com writes:
 On Tue, Jan 21, 2014 at 6:57 PM, MauMau maumau...@gmail.com wrote:
 To follow this, we have the line as:
 
 #event_source = 'PostgreSQL 9.4'
 
 But this requires us to change this line for each major release.  That's a
 maintenance headache.

 What I had in mind was to change it during initdb, we are already doing it
 for some other parameter (unix_socket_directories),

What happens when somebody copies their postgresql.conf from an older
version?  That's hardly uncommon, even though it might be considered bad
practice.  I don't think it's a good idea to try to insert a version
identifier this way.

But ... what's the point of including the PG version in this string
anyhow?  If you're trying to make the string unique across different
installations on the same machine, it's surely insufficient, and if
that's not the point then what is?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-21 Thread Amit Kapila
On Wed, Jan 22, 2014 at 9:19 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Amit Kapila amit.kapil...@gmail.com writes:
 On Tue, Jan 21, 2014 at 6:57 PM, MauMau maumau...@gmail.com wrote:
 To follow this, we have the line as:

 #event_source = 'PostgreSQL 9.4'

 But this requires us to change this line for each major release.  That's a
 maintenance headache.

 What I had in mind was to change it during initdb, we are already doing it
 for some other parameter (unix_socket_directories),

 What happens when somebody copies their postgresql.conf from an older
 version?  That's hardly uncommon, even though it might be considered bad
 practice.  I don't think it's a good idea to try to insert a version
 identifier this way.

 But ... what's the point of including the PG version in this string
 anyhow?  If you're trying to make the string unique across different
 installations on the same machine, it's surely insufficient, and if
 that's not the point then what is?

Well, certainly it cannot handle all different scenario's (particularly
same version installations), but the original report for this case was
for different versions of server. I think chances of having different
versions of server are much more, which will be handled by this
case. I felt even service name should include and we already have
Fixme in code for it, but thats separate patch altogether.
pg_ctl.c
..
(static char *register_servicename = PostgreSQL;
/* FIXME: + version ID? */


Also, I referred other s/w's which are registered for event source on
my m/c and I found it is common to include version number in some
form to distinguish different versions. For example, some of the
registered ones are:

Microsoft.Transactions.Bridge 3.0.0.0
Microsoft.Transactions.Bridge 4.0.0.0

ServiceModel Audit 3.0.0.0
ServiceModel Audit 4.0.0.0

I have also seen such a way to append versions to service names as
well.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-20 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com
  Do you think without this the problem reported by you is resolved 
completely.
  User can hit same problem, if he tries to follow similar steps mentioned 
in

  your first mail. I had tried below steps based on description in your
  first mail:

If user register/unregister different versions of pgevent.dll blindly,
then I think
any fix in this area could not prevent the error event source not found


OK, I'll add a check to prevent duplicate registration of the same event 
source with the message:


The specified event source is already registered.

Please correct me if there's better expression in English.

Are there any other suggestions to make this patch ready for committer?  I'd 
like to make all changes in one submission.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-20 Thread Amit Kapila
On Mon, Jan 20, 2014 at 5:38 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

   Do you think without this the problem reported by you is resolved
 completely.
   User can hit same problem, if he tries to follow similar steps mentioned
 in
   your first mail. I had tried below steps based on description in your
   first mail:

 If user register/unregister different versions of pgevent.dll blindly,
 then I think
 any fix in this area could not prevent the error event source not found


 OK, I'll add a check to prevent duplicate registration of the same event
 source with the message:

 The specified event source is already registered.

 Please correct me if there's better expression in English.

How about below message:

event source event_source_name is already registered.

This can make it more consistent with any other message in PG,
example create table.

 Are there any other suggestions to make this patch ready for committer?

Today, I reviewed the patch again and found it okay, except a small
inconsistency which is about default event source name in
postgresql.conf, all other places it's changed except in .conf file.
Do you think it makes sense to change there as well?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread Amit Kapila
On Thu, Dec 5, 2013 at 7:54 PM, MauMau maumau...@gmail.com wrote:
 Hello,

 I've removed a limitation regarding event log on Windows with the attached
 patch.  I hesitate to admit this is a bug fix and want to regard this an
 improvement, but maybe it's a bug fix from users' perspective.  Actually, I
 received problem reports from some users.


 [Problem]
 pg_ctl always uses event source PostgreSQL to output messages to the event
 log.  Some example messages are:

 server starting
 server started

 This causes the problems below:

 1. Cannot distinguish PostgreSQL instances because they use the same event
 source.

 2. If you use multiple installations of PostgreSQL on the same machine,
 whether they are the same or different versions, Windows event viewer
 reports an error event source not found in the following sequence.  Other
 system administration software which deal with event log may show other
 anomalies.
 2-1 Install the first PostgreSQL (e.g. 9.3) into dir_1, register
 dir_1\lib\pgevent.dll for event source PostgreSQL, then creates
 instance_1.
 2-2 Install the second PostgreSQL (e.g. 9.2) into dir_2 as part of some
 packaged application, register dir_2\lib\pgevent.dll for event source
 PostgreSQL,

Today, I was trying to reproduce this issue and found that if user tries
to register event source second time with same name, we just replace
the previous event source's path in registry.
Shouldn't we try to stop user at this step only, means if he tries to
register with same event source name more than once return error,
saying event source with same name already exists?


Another thing is after register of pgevent.dll, if I just try to start
PostgreSQL
it shows below messages in EventLog. Although the message has information
about server startup, but the start of Description is something similar to
what you were reporting event source not found.
Am I missing something?

Steps
1. installation of PostgreSQL from source code using Install.bat in
mdvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll
4. Modify postgresql.conf to set log_destination= 'eventlog'
5. pg_ctl.exe start -D ..\..\Data

Log Name:  Application
Source:PostgreSQL
Date:  1/19/2014 7:49:54 PM
Event ID:  0
Task Category: None
Level: Information
Keywords:  Classic
User:  N/A
Computer:  WIN-NGNN8PKIMD7
Description:
The description for Event ID 0 from source PostgreSQL cannot be found.
Either the component that raises this event is not installed on your
local computer or the installation is corrupted. You can install or
repair the component on the local computer.

If the event originated on another computer, the display information
had to be saved with the event.

The following information was included with the event:

LOG:  ending log output to stderr
HINT:  Future log output will go to log destination eventlog.


the message resource is present but the message is not found in the
string/message table


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Today, I was trying to reproduce this issue and found that if user tries
to register event source second time with same name, we just replace
the previous event source's path in registry.
Shouldn't we try to stop user at this step only, means if he tries to
register with same event source name more than once return error,
saying event source with same name already exists?


I'm OK with either.  If we add the check, I think that would be another 
patch.  However, I'm afraid the check won't be much effective, because the 
packaged application then unregister and register (i.e. regsvr32 /u and then 
regsvr32 /i) blindly.




Another thing is after register of pgevent.dll, if I just try to start
PostgreSQL
it shows below messages in EventLog. Although the message has information
about server startup, but the start of Description is something similar to
what you were reporting event source not found.
Am I missing something?

Steps
1. installation of PostgreSQL from source code using Install.bat in
mdvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll


Please specify pgevent.dll, not pgevent32.dll.

Regards
MauMau




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2014-01-19 Thread Amit Kapila
On Mon, Jan 20, 2014 at 4:05 AM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Today, I was trying to reproduce this issue and found that if user tries
 to register event source second time with same name, we just replace
 the previous event source's path in registry.
 Shouldn't we try to stop user at this step only, means if he tries to
 register with same event source name more than once return error,
 saying event source with same name already exists?


 I'm OK with either.  If we add the check, I think that would be another
 patch.

   Do you think without this the problem reported by you is resolved completely.
   User can hit same problem, if he tries to follow similar steps mentioned in
   your first mail. I had tried below steps based on description in your
   first mail:

   Steps
1. installation of PostgreSQL from source code (master) using Install.bat in
msvc directory
2. initdb -D data_dir
3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent.dll
4. Modify postgresql.conf to set log_destination= 'eventlog'
5. event_source = 'PostgreSQL'
6. pg_ctl.exe start -D ..\..\Data
7. psql -d postgres
8. Drop table t1; --try dropping some non-existant table
9. Check in Event viewer, you can find proper error.
10. NO Problem till above step.
11. Installation of PostgreSQL from source code (9.2) using Install.bat in
msvc directory
12. initdb -D 9.2_data_dir
13. regsvr32 /n /i:PostgreSQL install_9.2_dir_path\lib\pgevent.dll
14. Remove 9.2 installation (i have just renamed the install folder)
15. now perform step 8 again on master (with or without restart of server)
16. Open Event Viewer, you can see the message
 event source not found

Now this could have been avoided, if in step-13 we use different
event source name, but I think that will happen even without
this patch.

 However, I'm afraid the check won't be much effective, because the
 packaged application then unregister and register (i.e. regsvr32 /u and then
 regsvr32 /i) blindly.

If user register/unregister different versions of pgevent.dll blindly,
then I think
any fix in this area could not prevent the error event source not found


 Another thing is after register of pgevent.dll, if I just try to start
 PostgreSQL
 it shows below messages in EventLog. Although the message has information
 about server startup, but the start of Description is something similar to
 what you were reporting event source not found.
 Am I missing something?

 Steps
 1. installation of PostgreSQL from source code using Install.bat in
 mdvc directory
 2. initdb -D data_dir
 3. regsvr32 /n /i:PostgreSQL install_dir_path\lib\pgevent32.dll


 Please specify pgevent.dll, not pgevent32.dll.

Typo, I was using prevent.dll only, I think the reason is I need to restart
Event Viewer after register command.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-22 Thread Amit Kapila
On Fri, Dec 20, 2013 at 5:26 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Few other points:

 -
 1.
 #ifdef WIN32
 /* Get event source from postgresql.conf for eventlog output */
 get_config_value(event_source, event_source, sizeof(event_source));
 #endif

 event logging is done for both win32 and cygwin env.
 under hash define (Win32 || cygwin),
 so event source name should also be retrieved for both
 environments. Refer below in code:

 #if defined(WIN32) || defined(__CYGWIN__)
 static void
 write_eventlog(int level, const char *line)

 2.
 Docs needs to be updated for default value:
 http://www.postgresql.org/docs/devel/static/event-log-registration.html

 http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-EVENT-SOURCE


 Okay, done.  Thanks.  I'll update the commitfest entry this weekend.

Your changes are fine. The main part left from myside is test of this patch.
I will do that in next CF or If I get time before that, I will try to
complete it.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-20 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Few other points:
-
1.
#ifdef WIN32
/* Get event source from postgresql.conf for eventlog output */
get_config_value(event_source, event_source, sizeof(event_source));
#endif

event logging is done for both win32 and cygwin env.
under hash define (Win32 || cygwin),
so event source name should also be retrieved for both
environments. Refer below in code:

#if defined(WIN32) || defined(__CYGWIN__)
static void
write_eventlog(int level, const char *line)

2.
Docs needs to be updated for default value:
http://www.postgresql.org/docs/devel/static/event-log-registration.html
http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-EVENT-SOURCE


Okay, done.  Thanks.  I'll update the commitfest entry this weekend.

Regards
MauMau



pg_ctl_eventsrc_v4.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-17 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

Few minor things:
1.
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);

In this code, you are trying to access the value (*event_source) and
incase it is not initialised,
it will not contain the value and could cause problem, why not
directly check 'event_source'?


event_source here is a global static char array, so it's automatically 
initialized with zeros and safe to access.




2. minor coding style issue
pg_ctl.c
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);

elog.c
! evtHandle = RegisterEventSource(NULL,
! event_source ? event_source : DEFAULT_EVENT_SOURCE);

In both above usages, it is better that arguments in second line should 
start

inline with previous lines first argument. You can refer other places,
for ex. refer call to ReportEvent in pg_ctl.c just below
RegisterEventSource call.


Thanks.  I passed the source files through pgindent and attached the revised 
patch.  Although the arguments in the second line are not in line with the 
first line's arguments, that's what pgindent found good.


Regards
MauMau


pg_ctl_eventsrc_v3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-17 Thread Amit Kapila
On Tue, Dec 17, 2013 at 5:33 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 Few minor things:

 event_source here is a global static char array, so it's automatically
 initialized with zeros and safe to access.

   Right, I had missed that point.


 2. minor coding style issue


 Thanks.  I passed the source files through pgindent and attached the revised
 patch.  Although the arguments in the second line are not in line with the
 first line's arguments, that's what pgindent found good.

   Okay, no problem.

Few other points:
-
1.
#ifdef WIN32
/* Get event source from postgresql.conf for eventlog output */
get_config_value(event_source, event_source, sizeof(event_source));
#endif

event logging is done for both win32 and cygwin env.
under hash define (Win32 || cygwin),
so event source name should also be retrieved for both
environments. Refer below in code:

#if defined(WIN32) || defined(__CYGWIN__)
static void
write_eventlog(int level, const char *line)

2.
Docs needs to be updated for default value:
http://www.postgresql.org/docs/devel/static/event-log-registration.html
http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-EVENT-SOURCE

In this patch, we are planing to change default value of event_source
from PostgreSQL to PostgreSQL 9.4 (PostgreSQL PG_MAJORVERSION)
as part of fixing the issue reported in this thread.

If anyone has objection to that, please let us know now to avoid re-work
at later stage.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-15 Thread Amit Kapila
On Thu, Dec 12, 2013 at 4:43 PM, MauMau maumau...@gmail.com wrote:
 Hi, Amit san,

 From: Amit Kapila amit.kapil...@gmail.com

 [elog.c]
 Writing the default value in this file was redundant, because
 event_source
 cannot be NULL.  So change

 I think this change might not be safe as write_eventlog() gets called
 from write_stderr() which might get called
 before reading postgresql.conf, so the code as exists in PG might be
 to handle that case or some other similar
 situation where event_source is still not set. Have you confirmed in
 all ways that it is never the case that
 event_source is not set when the control reaches this function.


 Yes, you are right.  I considered this again after replying to your previous
 mail, and found out write_stderr() calls write_eventlog().  In that case,
 event_source is still NULL before GUC initialization.

Few minor things:
1.
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);

In this code, you are trying to access the value (*event_source) and
incase it is not initialised,
it will not contain the value and could cause problem, why not
directly check 'event_source'?

2. minor coding style issue
pg_ctl.c
evtHandle = RegisterEventSource(NULL,
*event_source? event_source: DEFAULT_EVENT_SOURCE);

elog.c
! evtHandle = RegisterEventSource(NULL,
! event_source ? event_source : DEFAULT_EVENT_SOURCE);

In both above usages, it is better that arguments in second line should start
inline with previous lines first argument. You can refer other places,
for ex. refer call to ReportEvent in pg_ctl.c just below
RegisterEventSource call.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-12 Thread MauMau

Hi, Amit san,

From: Amit Kapila amit.kapil...@gmail.com

[elog.c]
Writing the default value in this file was redundant, because 
event_source

cannot be NULL.  So change


I think this change might not be safe as write_eventlog() gets called
from write_stderr() which might get called
before reading postgresql.conf, so the code as exists in PG might be
to handle that case or some other similar
situation where event_source is still not set. Have you confirmed in
all ways that it is never the case that
event_source is not set when the control reaches this function.


Yes, you are right.  I considered this again after replying to your previous 
mail, and found out write_stderr() calls write_eventlog().  In that case, 
event_source is still NULL before GUC initialization.


Please find attached the patch.
I put the default event source name in one place -- pg_config_manual.h, 
which seems the best place.  This could eliminate duplicate default values 
in four places: pgevent.c, pg_ctl.c, elog.c, and guc.c.  Thanks to your 
review and comment, the code got cleaner and correct.  Thank you very much.


Regards
MauMau


pg_ctl_eventsrc_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-11 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

  I think it is better to keep it like what I suggested above,
because in that case
  it will assign default name even if postgres -C fails due to some 
reason.



2. What will happen if user doesn't change the name in event_source
or kept the same name,
   won't it hit the same problem again? So shouldn't it try to
generate different name by appending
   version string to it?




I re-considered that.  As you suggested, I think I'll do as follows.  Would 
this be OK?


[pg_ctl.c]
evtHandle = RegisterEventSource(NULL, *event_source ? event_source :
   PostgreSQL  PG_MAJORVERSION);


[guc.c]
{event_source, PGC_POSTMASTER, LOGGING_WHERE,
...
PostgreSQL  PG_MAJORVERSION,
NULL, NULL, NULL



[elog.c]
Writing the default value in this file was redundant, because event_source 
cannot be NULL.  So change


evtHandle = RegisterEventSource(NULL, event_source ? event_source : 
PostgreSQL);


to

evtHandle = RegisterEventSource(NULL, event_source);


Regards
MauMau




--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-11 Thread Amit Kapila
On Wed, Dec 11, 2013 at 8:31 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 I re-considered that.  As you suggested, I think I'll do as follows.  Would
 this be OK?

 [pg_ctl.c]
 evtHandle = RegisterEventSource(NULL, *event_source ? event_source :
PostgreSQL  PG_MAJORVERSION);


 [guc.c]
 {event_source, PGC_POSTMASTER, LOGGING_WHERE,
 ...
 PostgreSQL  PG_MAJORVERSION,
 NULL, NULL, NULL

   This is similar to what I had in mind.


 [elog.c]
 Writing the default value in this file was redundant, because event_source
 cannot be NULL.  So change


 evtHandle = RegisterEventSource(NULL, event_source ? event_source :
 PostgreSQL);

 to

 evtHandle = RegisterEventSource(NULL, event_source);

I think this change might not be safe as write_eventlog() gets called
from write_stderr() which might get called
before reading postgresql.conf, so the code as exists in PG might be
to handle that case or some other similar
situation where event_source is still not set. Have you confirmed in
all ways that it is never the case that
event_source is not set when the control reaches this function.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-09 Thread MauMau

From: Amit Kapila amit.kapil...@gmail.com

1. isn't it better to handle as it is done in write_eventlog() which
means if string is empty then
   use PostgreSQL.
evtHandle = RegisterEventSource(NULL, event_source ? event_source :
PostgreSQL);


Thank you for reviewing.  Yes, I did so with the first revision of this 
patch (see the first mail of this thread.)  I wanted to avoid duplicating 
the default value in both the server and pg_ctl code.  If user does not set 
event_source, postgres -C returns the default value PostgreSQL in the 
normal case, so I wanted to rely on it.  I thought the second revision would 
be appreciated by PostgreSQL-based products like EnterpriseDB, because there 
are fewer source files to modify.  But I don't mind which revision will be 
adopted.



2. What will happen if user doesn't change the name in event_source
or kept the same name,
   won't it hit the same problem again? So shouldn't it try to
generate different name by appending
   version string to it?


Yes, but I assume that the user has to set his own name to identify his 
instance uniquely.  Even if version string is added, the same issue can 
happen --- in the likely case where the user explicitly installs, for 
example, PostgreSQL 9.3 as a standalone database, as well as some packaged 
application that embeds PostgreSQL 9.3 which the user is unaware of.
If the user installs multiple versions of PostgreSQL explicitly with the 
community installer, the installer can set event_source = 'PostgreSQL 9.3' 
and 'PostgreSQL 9.4' for each instance.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-09 Thread Amit Kapila
On Mon, Dec 9, 2013 at 5:52 PM, MauMau maumau...@gmail.com wrote:
 From: Amit Kapila amit.kapil...@gmail.com

 1. isn't it better to handle as it is done in write_eventlog() which
 means if string is empty then
use PostgreSQL.
 evtHandle = RegisterEventSource(NULL, event_source ? event_source :
 PostgreSQL);


 Thank you for reviewing.  Yes, I did so with the first revision of this
 patch (see the first mail of this thread.)  I wanted to avoid duplicating
 the default value in both the server and pg_ctl code.  If user does not set
 event_source, postgres -C returns the default value PostgreSQL in the
 normal case, so I wanted to rely on it.

   I think it is better to keep it like what I suggested above,
because in that case
   it will assign default name even if postgres -C fails due to some reason.



 2. What will happen if user doesn't change the name in event_source
 or kept the same name,
won't it hit the same problem again? So shouldn't it try to
 generate different name by appending
version string to it?


 Yes, but I assume that the user has to set his own name to identify his
 instance uniquely.  Even if version string is added, the same issue can
 happen --- in the likely case where the user explicitly installs, for
 example, PostgreSQL 9.3 as a standalone database, as well as some packaged
 application that embeds PostgreSQL 9.3 which the user is unaware of.

   I mentioned it just to make sure that if such things can happen, it
is good to
   either avoid it or document it in some way, so that user can
understand better
   how to configure his system.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-08 Thread Magnus Hagander
On Sun, Dec 8, 2013 at 3:41 AM, MauMau maumau...@gmail.com wrote:

 From: MauMau maumau...@gmail.com

  I've removed a limitation regarding event log on Windows with the attached
 patch.  I hesitate to admit this is a bug fix and want to regard this an
 improvement, but maybe it's a bug fix from users' perspective.  Actually,
 I
 received problem reports from some users.


 I've done a small correction.  I removed redundant default value from the
 pg_ctl.c.


Not having looked at it in detail yet, but this seems to completely remove
the default value. What happens if the error that needs to be logged is the
one saying that it couldn't exec postgres to find out the value in the
config file? AFAICT it's going to try to register an eventsource with
whatever random garbage happens to be in the variable.

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


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-08 Thread MauMau

From: Magnus Hagander mag...@hagander.net

Not having looked at it in detail yet, but this seems to completely remove
the default value. What happens if the error that needs to be logged is 
the

one saying that it couldn't exec postgres to find out the value in the
config file? AFAICT it's going to try to register an eventsource with
whatever random garbage happens to be in the variable.


Thank you for commenting, Magnus san.
The variable is global and contains an empty string, so even in the unlikely 
situation where postgres -C fails, the event source simply becomes blank.


Regards
MauMau



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-08 Thread Amit Kapila
On Mon, Dec 9, 2013 at 2:25 AM, MauMau maumau...@gmail.com wrote:
 From: Magnus Hagander mag...@hagander.net

 Not having looked at it in detail yet, but this seems to completely remove
 the default value. What happens if the error that needs to be logged is
 the
 one saying that it couldn't exec postgres to find out the value in the
 config file? AFAICT it's going to try to register an eventsource with
 whatever random garbage happens to be in the variable.


 Thank you for commenting, Magnus san.
 The variable is global and contains an empty string, so even in the unlikely
 situation where postgres -C fails, the event source simply becomes blank.

1. isn't it better to handle as it is done in write_eventlog() which
means if string is empty then
use PostgreSQL.
evtHandle = RegisterEventSource(NULL, event_source ? event_source :
PostgreSQL);

2. What will happen if user doesn't change the name in event_source
or kept the same name,
won't it hit the same problem again? So shouldn't it try to
generate different name by appending
version string to it?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [bug fix] pg_ctl always uses the same event source

2013-12-07 Thread MauMau

From: MauMau maumau...@gmail.com

I've removed a limitation regarding event log on Windows with the attached
patch.  I hesitate to admit this is a bug fix and want to regard this an
improvement, but maybe it's a bug fix from users' perspective.  Actually, 
I

received problem reports from some users.


I've done a small correction.  I removed redundant default value from the 
pg_ctl.c.



Regards
MauMau



pg_ctl_eventsrc_v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers