pg_arch.c call to sleep()

Started by Andrew Dunstanabout 21 years ago9 messages
#1Andrew Dunstan
andrew@dunslane.net

We have the following warning on Windows:

pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace the call by a
call to pg_usleep().

I'm inclined to do the latter, on grounds of consistency, but want to be
sure that it will get the signal handling right.

Unless I hear squawks of objection I will supply a patch for that soon.

cheers

andrew

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: pg_arch.c call to sleep()

Andrew Dunstan <andrew@dunslane.net> writes:

We have the following warning on Windows:
pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace the call by a
call to pg_usleep().

<stdlib.h> is included automatically by c.h, so that surely won't fix it.

I have some recollection that we invented pg_usleep in part because we
wanted to not use sleep() at all in the backend, but I don't recall why
(and the reasoning might not apply to the archiver process, anyway).

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: pg_arch.c call to sleep()

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

We have the following warning on Windows:
pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace the call by a
call to pg_usleep().

<stdlib.h> is included automatically by c.h, so that surely won't fix it.

Now I look closer I see that it declares _sleep() rather than sleep().

I have some recollection that we invented pg_usleep in part because we
wanted to not use sleep() at all in the backend, but I don't recall why
(and the reasoning might not apply to the archiver process, anyway).

I thought we invented pg_usleep so we could remove the calls to
select(0,NULL,NULL,NULL,timeout) which don't work on Windows.

cheers

andrew

#4Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#2)
Re: pg_arch.c call to sleep()

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

We have the following warning on Windows:
pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace the call by a
call to pg_usleep().

<stdlib.h> is included automatically by c.h, so that surely won't fix it.

I have some recollection that we invented pg_usleep in part because we
wanted to not use sleep() at all in the backend, but I don't recall why
(and the reasoning might not apply to the archiver process, anyway).

I think usleep was so we could accept signals during the sleep.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#5Magnus Hagander
mha@sollentuna.net
In reply to: Bruce Momjian (#4)
Re: pg_arch.c call to sleep()

We have the following warning on Windows:
pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace

the call by

a call to pg_usleep().

<stdlib.h> is included automatically by c.h, so that surely
won't fix it.

I have some recollection that we invented pg_usleep in part
because we wanted to not use sleep() at all in the backend,
but I don't recall why (and the reasoning might not apply to
the archiver process, anyway).

win32 signal handling won't interrupt sleep(), just pg_usleep().

//Magnus

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Magnus Hagander (#5)
1 attachment(s)
Re: [HACKERS] pg_arch.c call to sleep()

Magnus Hagander wrote:

We have the following warning on Windows:
pgarch.c:349: warning: implicit declaration of function `sleep'

To fix it we could include the right header (which appears to be
<stdlib.h> in the Windows/Mingw case), or we could replace

the call by

a call to pg_usleep().

<stdlib.h> is included automatically by c.h, so that surely
won't fix it.

I have some recollection that we invented pg_usleep in part
because we wanted to not use sleep() at all in the backend,
but I don't recall why (and the reasoning might not apply to
the archiver process, anyway).

win32 signal handling won't interrupt sleep(), just pg_usleep().

I take this as confirmation that calling pg_usleep is the Right Thing (tm).

Here's the patch.

cheers

andrew

Attachments:

pgarch.patchtext/x-patch; name=pgarch.patchDownload
Index: pgarch.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.9
diff -c -r1.9 pgarch.c
*** pgarch.c	29 Aug 2004 05:06:46 -0000	1.9
--- pgarch.c	8 Nov 2004 12:59:49 -0000
***************
*** 338,352 ****
  
  		/*
  		 * There shouldn't be anything for the archiver to do except to
! 		 * wait for a signal, so we could use pause(3) here... ...however,
! 		 * the archiver exists to protect our data, so she wakes up
! 		 * occasionally to allow herself to be proactive. In particular
! 		 * this avoids getting stuck if a signal arrives just before we
! 		 * enter sleep().
  		 */
  		if (!wakened)
  		{
! 			sleep(PGARCH_AUTOWAKE_INTERVAL);
  
  			curtime = time(NULL);
  			if ((unsigned int) (curtime - last_copy_time) >=
--- 338,351 ----
  
  		/*
  		 * There shouldn't be anything for the archiver to do except to
! 		 * wait for a signal, ... however, the archiver exists to 
! 		 * protect our data, so she wakes up occasionally to allow 
! 		 * herself to be proactive. In particular this avoids getting 
! 		 * stuck if a signal arrives just before we sleep.
  		 */
  		if (!wakened)
  		{
! 			pg_usleep(PGARCH_AUTOWAKE_INTERVAL * 1000000L);
  
  			curtime = time(NULL);
  			if ((unsigned int) (curtime - last_copy_time) >=
#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#6)
Re: [HACKERS] pg_arch.c call to sleep()

Andrew Dunstan <andrew@dunslane.net> writes:

I take this as confirmation that calling pg_usleep is the Right Thing (tm).
Here's the patch.

Applied.

regards, tom lane

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#7)
1 attachment(s)
Re: [HACKERS] pg_arch.c call to sleep()

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

I take this as confirmation that calling pg_usleep is the Right Thing (tm).
Here's the patch.

Applied.

Darnit, I caught one and not the other. Here's a oneline fix.

cheers

andrew

Attachments:

pgarch2.patchtext/x-patch; name=pgarch2.patchDownload
Index: src/backend/postmaster/pgarch.c
===================================================================
RCS file: /home/cvsmirror/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.11
diff -c -r1.11 pgarch.c
*** src/backend/postmaster/pgarch.c	17 Nov 2004 17:50:20 -0000	1.11
--- src/backend/postmaster/pgarch.c	18 Nov 2004 16:57:10 -0000
***************
*** 392,398 ****
  									xlog)));
  					return;		/* give up archiving for now */
  				}
! 				sleep(1);		/* wait a bit before retrying */
  			}
  		}
  	}
--- 392,398 ----
  									xlog)));
  					return;		/* give up archiving for now */
  				}
! 				pg_usleep(1000000L);	/* wait 1 sec before retrying */
  			}
  		}
  	}
#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#8)
Re: [HACKERS] pg_arch.c call to sleep()

Andrew Dunstan <andrew@dunslane.net> writes:

Darnit, I caught one and not the other. Here's a oneline fix.

Done.

regards, tom lane