BUG #4961: pg_standby.exe crashes with no args

Started by Nonameover 16 years ago20 messages
#1Noname
wader2@jcom.home.ne.jp

The following bug has been logged online:

Bug reference: 4961
Logged by:
Email address: wader2@jcom.home.ne.jp
PostgreSQL version: 8.4.0
Operating system: Windows XP/2000
Description: pg_standby.exe crashes with no args
Details:

also clashes when run with recovery.conf,
after logged message below.

could not restore file "00000001.history" from archive: return code
-1073741811

#2Fujii Masao
masao.fujii@gmail.com
In reply to: Noname (#1)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi,

On Mon, Aug 3, 2009 at 9:18 PM, <wader2@jcom.home.ne.jp> wrote:

also clashes when run with recovery.conf,
after logged message below.

could not restore file "00000001.history" from archive: return code
-1073741811

Does pg_standby (e.g., pg_standby.exe --version) succeed when you
execute it alone? If not, you might have failed the installation of it.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#3wader2
wader2@jcom.home.ne.jp
In reply to: Fujii Masao (#2)
Re: BUG #4961: pg_standby.exe crashes with no args

thx,

Does pg_standby (e.g., pg_standby.exe --version) succeed when you

--version and --help are no problem.

#4Bruce Momjian
bruce@momjian.us
In reply to: wader2 (#3)
Re: BUG #4961: pg_standby.exe crashes with no args

wader2 wrote:

thx,

Does pg_standby (e.g., pg_standby.exe --version) succeed when you

--version and --help are no problem.

I can't reproduce a crash here on BSD:

$ pg_standby
pg_standby: not enough command-line arguments

Can you show us the command and the crash text?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

#5wader2
wader2@jcom.home.ne.jp
In reply to: Bruce Momjian (#4)
Re: BUG #4961: pg_standby.exe crashes with no args

Bruce Momjian wrote:

I can't reproduce a crash here on BSD:

$ pg_standby
pg_standby: not enough command-line arguments

Can you show us the command and the crash text?

I guess this occurs on only windows (Japanese envionment?).

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe

results no text on command line, Windows error dialog.

AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
ModVer:8.0.50727.762 Offset:000091ad

#6Hiroshi Saito
z-saito@guitar.ocn.ne.jp
In reply to: Bruce Momjian (#4)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi.

Yes, I also reproduce it. It is very strange......
==
Windows-XP Home Edition Version2002 Service Pack3(Japanese)
CPU N270 @ 1.60Ghz 1GB RAM
==

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe --help
pg_standby allows PostgreSQL warm standby servers to be configured.

Usage:
<snip>

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe
...crash...

However, MinGW+gcc returns a normal result.

$ pg_standby
pg_standby: not enough command-line arguments

I don't have the margin time which still investigates it.

Regards,
Hiroshi Saito

----- Original Message -----
From: "wader2" <wader2@jcom.home.ne.jp>

Show quoted text

Bruce Momjian wrote:

I can't reproduce a crash here on BSD:

$ pg_standby
pg_standby: not enough command-line arguments

Can you show us the command and the crash text?

I guess this occurs on only windows (Japanese envionment?).

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe

results no text on command line, Windows error dialog.

AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
ModVer:8.0.50727.762 Offset:000091ad

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

#7Magnus Hagander
magnus@hagander.net
In reply to: wader2 (#5)
Re: BUG #4961: pg_standby.exe crashes with no args

On Mon, Aug 10, 2009 at 16:10, wader2<wader2@jcom.home.ne.jp> wrote:

Bruce Momjian wrote:

I can't reproduce a crash here on BSD:

       $ pg_standby
       pg_standby: not enough command-line arguments

Can you show us the command and the crash text?

I guess this occurs on only windows (Japanese envionment?).

C:\Program Files\PostgreSQL\8.4\bin>pg_standby.exe

results no text on command line, Windows error dialog.

AppName:pg_standby.exe AppVer:0.0.0.0 ModName:msvcr80.dll
ModVer:8.0.50727.762   Offset:000091ad

I have reproduced this. The problem is:
(void) signal(SIGUSR1, sighandler);
(void) signal(SIGINT, sighandler); /* deprecated, use SIGUSR1 */

None of these signals exist on WIN32. I think the only reason it
compiles at all is that we bring in *some* of our signals emulation
code, but certainly not all of it.

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#7)
Re: [HACKERS] BUG #4961: pg_standby.exe crashes with no args

Magnus Hagander <magnus@hagander.net> writes:

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

It would mean there's no way to trigger failover via signal.

I think what we need is for pg_ctl to be able to send these signals...

regards, tom lane

#9Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#8)
Re: [HACKERS] BUG #4961: pg_standby.exe crashes with no args

On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

It would mean there's no way to trigger failover via signal.

I think what we need is for pg_ctl to be able to send these signals...

Those signals don't *exist* on Windows. The whole idea of
cross-process signals don't *exist* on Windows.

We emulate it in the main backend, by creating a background thread
that sets a global variable. That is then polled in the
CHECK_FOR_INTERRUPTS macro. pg_ctl is perfectly capable of sending
these signals, but pg_standby can't receive them.

We could implement the same type of check in pg_standby, but it
requires something like CHECK_FOR_INTERRUPTS. And these interrupts
won't, by default, cause any kind of interruption of the process. In
the backend, we interrupt socket calls because we have the socket
wrapper layer, and nothing else. I don't know how doable this would be
in pg_standby - does it always block on a single thing where we could
stick some win32 synchronization code? If it's a single, or limited,
places we could implement something similar to the backend. But if we
need to interrupt at arbitrary locations, that's just not possible.

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

#10wader2
wader2@jcom.home.ne.jp
In reply to: Magnus Hagander (#9)
Re: [HACKERS] BUG #4961: pg_standby.exe crashes with no args

I can't compile nor read source, but can tell you that
pg_standby.exe in 8.3.7 works fine.

Magnus Hagander wrote:

Show quoted text

On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

It would mean there's no way to trigger failover via signal.

I think what we need is for pg_ctl to be able to send these signals...

Those signals don't *exist* on Windows. The whole idea of
cross-process signals don't *exist* on Windows.

We emulate it in the main backend, by creating a background thread
that sets a global variable. That is then polled in the
CHECK_FOR_INTERRUPTS macro. pg_ctl is perfectly capable of sending
these signals, but pg_standby can't receive them.

We could implement the same type of check in pg_standby, but it
requires something like CHECK_FOR_INTERRUPTS. And these interrupts
won't, by default, cause any kind of interruption of the process. In
the backend, we interrupt socket calls because we have the socket
wrapper layer, and nothing else. I don't know how doable this would be
in pg_standby - does it always block on a single thing where we could
stick some win32 synchronization code? If it's a single, or limited,
places we could implement something similar to the backend. But if we
need to interrupt at arbitrary locations, that's just not possible.

#11Magnus Hagander
magnus@hagander.net
In reply to: wader2 (#10)
Re: [HACKERS] BUG #4961: pg_standby.exe crashes with no args

Yeah, the patch I think breaks things isn't included in 8.3.7 - it
will be in 8.3.8 though...

//Magnus

On Wed, Aug 12, 2009 at 11:08, wader2<wader2@jcom.home.ne.jp> wrote:

I can't compile nor read source, but can tell you that
pg_standby.exe in 8.3.7 works fine.

Magnus Hagander wrote:

On Mon, Aug 10, 2009 at 20:44, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

If I just move those two lines into the #ifndef WIN32 block just
around it, it compiles and doesn't crash on running-with-no-arguments.
I haven't tried to actually use it though - can someone confirm if
this will actually make pg_standby not work properly?

It would mean there's no way to trigger failover via signal.

I think what we need is for pg_ctl to be able to send these signals...

Those signals don't *exist* on Windows. The whole idea of
cross-process signals don't *exist* on Windows.

We emulate it in the main backend, by creating a background thread
that sets a global variable. That is then polled in the
CHECK_FOR_INTERRUPTS macro.  pg_ctl is perfectly capable of sending
these signals, but pg_standby can't receive them.

We could implement the same type of check in pg_standby, but it
requires something like CHECK_FOR_INTERRUPTS. And these interrupts
won't, by default, cause any kind of interruption of the process. In
the backend, we interrupt socket calls because we have the socket
wrapper layer, and nothing else. I don't know how doable this would be
in pg_standby - does it always block on a single thing where we could
stick some win32 synchronization code? If it's a single, or limited,
places we could implement something similar to the backend. But if we
need to interrupt at arbitrary locations, that's just not possible.

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

#12Fujii Masao
masao.fujii@gmail.com
In reply to: Magnus Hagander (#7)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi,

On Tue, Aug 11, 2009 at 3:10 AM, Magnus Hagander<magnus@hagander.net> wrote:

I have reproduced this. The problem is:
       (void) signal(SIGUSR1, sighandler);
       (void) signal(SIGINT, sighandler);      /* deprecated, use SIGUSR1 */

None of these signals exist on WIN32. I think the only reason it
compiles at all is that we bring in *some* of our signals emulation
code, but certainly not all of it.

SIGINT has been used in pg_standby for a while (e.g., v8.3.7). I wonder
why this problem arises only in v8.4.0.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#13Magnus Hagander
magnus@hagander.net
In reply to: Fujii Masao (#12)
Re: BUG #4961: pg_standby.exe crashes with no args

On Wed, Aug 12, 2009 at 19:08, Fujii Masao<masao.fujii@gmail.com> wrote:

Hi,

On Tue, Aug 11, 2009 at 3:10 AM, Magnus Hagander<magnus@hagander.net> wrote:

I have reproduced this. The problem is:
       (void) signal(SIGUSR1, sighandler);
       (void) signal(SIGINT, sighandler);      /* deprecated, use SIGUSR1 */

None of these signals exist on WIN32. I think the only reason it
compiles at all is that we bring in *some* of our signals emulation
code, but certainly not all of it.

SIGINT has been used in pg_standby for a while (e.g., v8.3.7). I wonder
why this problem arises only in v8.4.0.

Not sure. Potentially pure luck. SIGINT has never *worked*, though, it
just hasn't crashed.

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

#14Fujii Masao
masao.fujii@gmail.com
In reply to: Magnus Hagander (#13)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi,

On Thu, Aug 13, 2009 at 2:24 AM, Magnus Hagander<magnus@hagander.net> wrote:

Not sure. Potentially pure luck. SIGINT has never *worked*, though, it
just hasn't crashed.

OK.

We could implement the same type of check in pg_standby, but it
requires something like CHECK_FOR_INTERRUPTS. And these interrupts
won't, by default, cause any kind of interruption of the process. In
the backend, we interrupt socket calls because we have the socket
wrapper layer, and nothing else. I don't know how doable this would be
in pg_standby - does it always block on a single thing where we could
stick some win32 synchronization code? If it's a single, or limited,
places we could implement something similar to the backend. But if we
need to interrupt at arbitrary locations, that's just not possible.

I think that CHECK_FOR_INTERRUPTS should be placed just before
checking the flag 'signaled' which may be enabled by the signal handler.
Here is the pseudo-code.

--------------------
{
/* Check for trigger file or signal first */
CheckForExternalTrigger();
+ #ifdef WIN32
+ CHECK_FOR_INTERRUPTS();
+ #endif /* WIN32 */
if (signaled)
{
Failover = FastFailover;
--------------------

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#15Magnus Hagander
magnus@hagander.net
In reply to: Fujii Masao (#14)
Re: BUG #4961: pg_standby.exe crashes with no args

On Wed, Aug 19, 2009 at 08:38, Fujii Masao<masao.fujii@gmail.com> wrote:

On Thu, Aug 13, 2009 at 2:24 AM, Magnus Hagander<magnus@hagander.net> wrote:

Not sure. Potentially pure luck. SIGINT has never *worked*, though, it
just hasn't crashed.

OK.

We could implement the same type of check in pg_standby, but it
requires something like CHECK_FOR_INTERRUPTS. And these interrupts
won't, by default, cause any kind of interruption of the process. In
the backend, we interrupt socket calls because we have the socket
wrapper layer, and nothing else. I don't know how doable this would be
in pg_standby - does it always block on a single thing where we could
stick some win32 synchronization code? If it's a single, or limited,
places we could implement something similar to the backend. But if we
need to interrupt at arbitrary locations, that's just not possible.

I think that CHECK_FOR_INTERRUPTS should be placed just before
checking the flag 'signaled' which may be enabled by the signal handler.
Here is the pseudo-code.

--------------------
       {
               /* Check for trigger file or signal first */
               CheckForExternalTrigger();
+ #ifdef WIN32
+               CHECK_FOR_INTERRUPTS();
+ #endif /* WIN32 */
               if (signaled)
               {
                       Failover = FastFailover;
--------------------

It's going to take a lot more than that, really. I looked a bit more
at it, and I think the best way is to do a win32 specific thing all
the way, not just emulate signals. Given that we only have a couple of
signal() calls anyway. I think that's going to be much clearer in what
it does, and also a lot simpler code in the end. The reason we have
the full emulation layer is that we use signals all over the place in
the backend.

From what I can tell, we don't actually need to interrupt anything
other than the sleep call on line 810, because we only check for the
signaled=true state in that loop anyway. Can someone more
knowledgeable in the pg_standby code comment on that?

This would amount to fairly major surgery for pg_standby on Win32. Is
that something we'd want to backpatch, or do we want to backpatch just
the removal of the signal() calls which would amount to not supporting
signals in pg_standby on win32?

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

#16Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Magnus Hagander (#15)
Re: BUG #4961: pg_standby.exe crashes with no args

Magnus Hagander wrote:

This would amount to fairly major surgery for pg_standby on Win32. Is
that something we'd want to backpatch, or do we want to backpatch just
the removal of the signal() calls which would amount to not supporting
signals in pg_standby on win32?

I think we should just remove the signals support for win32. The trigger
file method still works, and the signal method has always been a bit
iffy (it doesn't work when pg_standby isn't running, for example, which
happens between processing of each WAL file, because there's no process
to signal).

Is pg_standby killed correctly when postmaster dies?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#17Magnus Hagander
magnus@hagander.net
In reply to: Heikki Linnakangas (#16)
Re: BUG #4961: pg_standby.exe crashes with no args

On Wed, Aug 19, 2009 at 11:45, Heikki
Linnakangas<heikki.linnakangas@enterprisedb.com> wrote:

Magnus Hagander wrote:

This would amount to fairly major surgery for pg_standby on Win32. Is
that something we'd want to backpatch, or do we want to backpatch just
the removal of the signal() calls which would amount to not supporting
signals in pg_standby on win32?

I think we should just remove the signals support for win32. The trigger
file method still works, and the signal method has always been a bit
iffy (it doesn't work when pg_standby isn't running, for example, which
happens between processing of each WAL file, because there's no process
to signal).

Fair enough.

Is pg_standby killed correctly when postmaster dies?

No idea. I can set up and env to test, but I don't have one ready.
Anybody else have tried this?

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

#18Fujii Masao
masao.fujii@gmail.com
In reply to: Heikki Linnakangas (#16)
1 attachment(s)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi,

Sorry for this late reply.

On Wed, Aug 19, 2009 at 6:45 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Is pg_standby killed correctly when postmaster dies?

No. In my test (v8.3.7 on Win), an immediate shutdown was able to
kill postmaster, but not pg_standby and the startup process.

Though we should change those to handle the shutdown signals
correctly, this change is not imperative need. At first, we should
just get rid of the signal support from pg_standby on Win, in order
to avoid the pg_standby crash. The attached is the patch to do so.
If this patch is OK, the backport is required for 8.3.x.

Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

drop_signal_support_for_pgstandby_win32.patchtext/x-patch; charset=US-ASCII; name=drop_signal_support_for_pgstandby_win32.patchDownload
? drop_signal_support_for_pgstandby_win32.patch
Index: contrib/pg_standby/pg_standby.c
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/pg_standby/pg_standby.c,v
retrieving revision 1.26
diff -c -r1.26 pg_standby.c
*** contrib/pg_standby/pg_standby.c	25 Jun 2009 19:33:25 -0000	1.26
--- contrib/pg_standby/pg_standby.c	4 Nov 2009 10:27:26 -0000
***************
*** 56,62 ****
--- 56,64 ----
  bool		need_cleanup = false;		/* do we need to remove files from
  										 * archive? */
  
+ #ifndef WIN32
  static volatile sig_atomic_t signaled = false;
+ #endif
  
  char	   *archiveLocation;	/* where to find the archive? */
  char	   *triggerPath;		/* where to find the trigger file? */
***************
*** 535,547 ****
  	printf("\nReport bugs to <pgsql-bugs@postgresql.org>.\n");
  }
  
  static void
  sighandler(int sig)
  {
  	signaled = true;
  }
  
- #ifndef WIN32
  /* We don't want SIGQUIT to core dump */
  static void
  sigquit_handler(int sig)
--- 537,549 ----
  	printf("\nReport bugs to <pgsql-bugs@postgresql.org>.\n");
  }
  
+ #ifndef WIN32
  static void
  sighandler(int sig)
  {
  	signaled = true;
  }
  
  /* We don't want SIGQUIT to core dump */
  static void
  sigquit_handler(int sig)
***************
*** 573,578 ****
--- 575,581 ----
  		}
  	}
  
+ #ifndef WIN32
  	/*
  	 * You can send SIGUSR1 to trigger failover.
  	 *
***************
*** 584,593 ****
  	 * out to be a bad idea because postmaster uses SIGQUIT to request
  	 * immediate shutdown. We still trap SIGINT, but that may change in a
  	 * future release.
  	 */
  	(void) signal(SIGUSR1, sighandler);
  	(void) signal(SIGINT, sighandler);	/* deprecated, use SIGUSR1 */
- #ifndef WIN32
  	(void) signal(SIGQUIT, sigquit_handler);
  #endif
  
--- 587,597 ----
  	 * out to be a bad idea because postmaster uses SIGQUIT to request
  	 * immediate shutdown. We still trap SIGINT, but that may change in a
  	 * future release.
+ 	 *
+ 	 * There's no way to trigger failover via signal on Windows.
  	 */
  	(void) signal(SIGUSR1, sighandler);
  	(void) signal(SIGINT, sighandler);	/* deprecated, use SIGUSR1 */
  	(void) signal(SIGQUIT, sigquit_handler);
  #endif
  
***************
*** 763,768 ****
--- 767,773 ----
  	{
  		/* Check for trigger file or signal first */
  		CheckForExternalTrigger();
+ #ifndef WIN32
  		if (signaled)
  		{
  			Failover = FastFailover;
***************
*** 772,777 ****
--- 777,783 ----
  				fflush(stderr);
  			}
  		}
+ #endif
  
  		/*
  		 * Check for fast failover immediately, before checking if the
#19Heikki Linnakangas
heikki.linnakangas@enterprisedb.com
In reply to: Fujii Masao (#18)
Re: BUG #4961: pg_standby.exe crashes with no args

Fujii Masao wrote:

On Wed, Aug 19, 2009 at 6:45 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Is pg_standby killed correctly when postmaster dies?

No. In my test (v8.3.7 on Win), an immediate shutdown was able to
kill postmaster, but not pg_standby and the startup process.

Hmm. We should do something about that.. Magnus?

Though we should change those to handle the shutdown signals
correctly, this change is not imperative need. At first, we should
just get rid of the signal support from pg_standby on Win, in order
to avoid the pg_standby crash. The attached is the patch to do so.
If this patch is OK, the backport is required for 8.3.x.

Ok, committed to HEAD, 8.4 and 8.3.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

#20Fujii Masao
masao.fujii@gmail.com
In reply to: Heikki Linnakangas (#19)
Re: BUG #4961: pg_standby.exe crashes with no args

Hi,

On Wed, Nov 4, 2009 at 9:53 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Though we should change those to handle the shutdown signals
correctly, this change is not imperative need. At first, we should
just get rid of the signal support from pg_standby on Win, in order
to avoid the pg_standby crash. The attached is the patch to do so.
If this patch is OK, the backport is required for 8.3.x.

Ok, committed to HEAD, 8.4 and 8.3.

Thanks a lot!

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center