Tom Lane wrote:
> Joe Conway <[EMAIL PROTECTED]> writes:
> > [ some convincing test cases that timeout=1 is not good ]
>
> > remains.tv_sec = atoi(conn->connect_timeout);
> > + if (remains.tv_sec == 1)
> > + remains.tv_sec += 1;
> > if (!remains.tv_sec)
> > {
> > conn->status = CONNECTION_BAD;
>
> On pure-paranoia grounds, I'd suggest the logic
>
> + /* force a sane minimum delay */
> + if (remains.tv_sec < 2)
> + remains.tv_sec = 2;
>
> whereupon you could remove the failure check just below.
I think we should fail if they set the timeout to zero, rather than
cover it up by setting the delay to two.
Attached is a patch that implements most of what we have discussed:
use time()
get rid of timeval where not needed
allow restart of select() to properly compute remaining time
add 1 to timeout == 1
pass finish time to pqWaitTimed
Patch applied. I am applying it so it is in CVS and everyone can see
it. I will keep modifying it until everyone likes it. It is just
easier to do it that way when multiple people are reviewing it. They
can jump in and make changes too.
--
Bruce Momjian | http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.210
diff -c -c -r1.210 fe-connect.c
*** src/interfaces/libpq/fe-connect.c 15 Oct 2002 01:48:25 -0000 1.210
--- src/interfaces/libpq/fe-connect.c 16 Oct 2002 02:48:07 -0000
***************
*** 1052,1061 ****
{
PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
! time_t finish_time = 0,
! current_time;
! struct timeval remains,
! *rp = NULL;
if (conn == NULL || conn->status == CONNECTION_BAD)
return 0;
--- 1052,1058 ----
{
PostgresPollingStatusType flag = PGRES_POLLING_WRITING;
! time_t finish_time = -1;
if (conn == NULL || conn->status == CONNECTION_BAD)
return 0;
***************
*** 1065,1084 ****
*/
if (conn->connect_timeout != NULL)
{
! remains.tv_sec = atoi(conn->connect_timeout);
! if (!remains.tv_sec)
{
conn->status = CONNECTION_BAD;
return 0;
}
! remains.tv_usec = 0; /* We don't use subsecond timing */
! rp = &remains;
!
/* calculate the finish time based on start + timeout */
! finish_time = time((time_t *) NULL) + remains.tv_sec;
}
! while (rp == NULL || remains.tv_sec > 0)
{
/*
* Wait, if necessary. Note that the initial state (just after
--- 1062,1082 ----
*/
if (conn->connect_timeout != NULL)
{
! int timeout = atoi(conn->connect_timeout);
!
! if (timeout == 0)
{
conn->status = CONNECTION_BAD;
return 0;
}
! /* Rounding could cause connection to fail;we need at least 2 secs */
! if (timeout == 1)
! timeout++;
/* calculate the finish time based on start + timeout */
! finish_time = time(NULL) + timeout;
}
! while (finish_time == -1 || time(NULL) >= finish_time)
{
/*
* Wait, if necessary. Note that the initial state (just after
***************
*** 1094,1100 ****
return 1; /* success! */
case PGRES_POLLING_READING:
! if (pqWaitTimed(1, 0, conn, rp))
{
conn->status = CONNECTION_BAD;
return 0;
--- 1092,1098 ----
return 1; /* success! */
case PGRES_POLLING_READING:
! if (pqWaitTimed(1, 0, conn, finish_time))
{
conn->status = CONNECTION_BAD;
return 0;
***************
*** 1102,1108 ****
break;
case PGRES_POLLING_WRITING:
! if (pqWaitTimed(0, 1, conn, rp))
{
conn->status = CONNECTION_BAD;
return 0;
--- 1100,1106 ----
break;
case PGRES_POLLING_WRITING:
! if (pqWaitTimed(0, 1, conn, finish_time))
{
conn->status = CONNECTION_BAD;
return 0;
***************
*** 1119,1138 ****
* Now try to advance the state machine.
*/
flag = PQconnectPoll(conn);
-
- /*
- * If connecting timeout is set, calculate remaining time.
- */
- if (rp != NULL)
- {
- if (time(¤t_time) == -1)
- {
- conn->status = CONNECTION_BAD;
- return 0;
- }
-
- remains.tv_sec = finish_time - current_time;
- }
}
conn->status = CONNECTION_BAD;
return 0;
--- 1117,1122 ----
Index: src/interfaces/libpq/fe-misc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/fe-misc.c,v
retrieving revision 1.83
diff -c -c -r1.83 fe-misc.c
*** src/interfaces/libpq/fe-misc.c 14 Oct 2002 18:11:17 -0000 1.83
--- src/interfaces/libpq/fe-misc.c 16 Oct 2002 02:48:10 -0000
***************
*** 779,789 ****
int
pqWait(int forRead, int forWrite, PGconn *conn)
{
! return pqWaitTimed(forRead, forWrite, conn, (const struct timeval *) NULL);
}
int
! pqWaitTimed(int forRead, int forWrite, PGconn *conn, const struct timeval *timeout)
{
fd_set input_mask;
fd_set output_mask;
--- 779,789 ----
int
pqWait(int forRead, int forWrite, PGconn *conn)
{
! return pqWaitTimed(forRead, forWrite, conn, -1);
}
int
! pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
{
fd_set input_mask;
fd_set output_mask;
***************
*** 820,826 ****
FD_SET(conn->sock, &output_mask);
FD_SET(conn->sock, &except_mask);
! if (NULL != timeout)
{
/*
* select() may modify timeout argument on some platforms
so
--- 820,826 ----
FD_SET(conn->sock, &output_mask);
FD_SET(conn->sock, &except_mask);
! if (finish_time != -1)
{
/*
* select() may modify timeout argument on some platforms
so
***************
*** 831,837 ****
* incorrect timings when select() is interrupted.
* bjm 2002-10-14
*/
! tmp_timeout = *timeout;
ptmp_timeout = &tmp_timeout;
}
if (select(conn->sock + 1, &input_mask, &output_mask,
--- 831,839 ----
* incorrect timings when select() is interrupted.
* bjm 2002-10-14
*/
! if ((tmp_timeout.tv_sec = finish_time - time(NULL)) < 0)
! tmp_timeout.tv_sec = 0; /* possible? */
! tmp_timeout.tv_usec = 0;
ptmp_timeout = &tmp_timeout;
}
if (select(conn->sock + 1, &input_mask, &output_mask,
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.59
diff -c -c -r1.59 libpq-int.h
*** src/interfaces/libpq/libpq-int.h 14 Oct 2002 17:15:11 -0000 1.59
--- src/interfaces/libpq/libpq-int.h 16 Oct 2002 02:48:12 -0000
***************
*** 340,346 ****
extern int pqFlush(PGconn *conn);
extern int pqSendSome(PGconn *conn);
extern int pqWait(int forRead, int forWrite, PGconn *conn);
! extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn, const struct
timeval *timeout);
extern int pqReadReady(PGconn *conn);
extern int pqWriteReady(PGconn *conn);
--- 340,347 ----
extern int pqFlush(PGconn *conn);
extern int pqSendSome(PGconn *conn);
extern int pqWait(int forRead, int forWrite, PGconn *conn);
! extern int pqWaitTimed(int forRead, int forWrite, PGconn *conn,
! time_t finish_time);
extern int pqReadReady(PGconn *conn);
extern int pqWriteReady(PGconn *conn);
---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
