Thread: Re: implementing query timeout

Here is my first draft of a query timeout SET variable.  It works for a
simple test:

    test=> set query_timeout to '2000';
    SET
    test=> select * from pg_class, pg_type;
    ERROR:  Query was cancelled.
    test=>

I still need to polish it up and do more testing.  Can people comment on
the proper placement of the disable_sig_alarm(true) calls?  Also, the
handling of the alarm is tricky because the deadlock timer uses the
alarm as well and the query_timeout.  (I have not gotten all the cases
correct yet.)

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.120
diff -c -r1.120 runtime.sgml
*** doc/src/sgml/runtime.sgml    5 Jul 2002 01:17:20 -0000    1.120
--- doc/src/sgml/runtime.sgml    10 Jul 2002 03:58:52 -0000
***************
*** 1585,1590 ****
--- 1585,1600 ----
       </varlistentry>

       <varlistentry>
+       <term><varname>QUERY_TIMEOUT</varname> (<type>integer</type>)</term>
+       <listitem>
+        <para>
+         Aborts any query that takes over the specified number of
+         microseconds.  A value of zero turns off the timer.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><varname>SHARED_BUFFERS</varname> (<type>integer</type>)</term>
        <listitem>
         <para>
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.280
diff -c -r1.280 postmaster.c
*** src/backend/postmaster/postmaster.c    20 Jun 2002 20:29:33 -0000    1.280
--- src/backend/postmaster/postmaster.c    10 Jul 2002 03:58:56 -0000
***************
*** 2105,2111 ****
       * after a time delay, so that a broken client can't hog a connection
       * indefinitely.  PreAuthDelay doesn't count against the time limit.
       */
!     if (!enable_sigalrm_interrupt(AuthenticationTimeout * 1000))
          elog(FATAL, "DoBackend: Unable to set timer for auth timeout");

      /*
--- 2105,2111 ----
       * after a time delay, so that a broken client can't hog a connection
       * indefinitely.  PreAuthDelay doesn't count against the time limit.
       */
!     if (!enable_sig_alarm(AuthenticationTimeout * 1000, false))
          elog(FATAL, "DoBackend: Unable to set timer for auth timeout");

      /*
***************
*** 2134,2140 ****
       * Done with authentication.  Disable timeout, and prevent
       * SIGTERM/SIGQUIT again until backend startup is complete.
       */
!     if (!disable_sigalrm_interrupt())
          elog(FATAL, "DoBackend: Unable to disable timer for auth timeout");
      PG_SETMASK(&BlockSig);

--- 2134,2140 ----
       * Done with authentication.  Disable timeout, and prevent
       * SIGTERM/SIGQUIT again until backend startup is complete.
       */
!     if (!disable_sig_alarm(false))
          elog(FATAL, "DoBackend: Unable to disable timer for auth timeout");
      PG_SETMASK(&BlockSig);

Index: src/backend/storage/lmgr/proc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v
retrieving revision 1.121
diff -c -r1.121 proc.c
*** src/backend/storage/lmgr/proc.c    20 Jun 2002 20:29:35 -0000    1.121
--- src/backend/storage/lmgr/proc.c    10 Jul 2002 03:58:56 -0000
***************
*** 52,59 ****
  #include "storage/sinval.h"
  #include "storage/spin.h"

-
  int            DeadlockTimeout = 1000;

  PGPROC       *MyProc = NULL;

--- 52,61 ----
  #include "storage/sinval.h"
  #include "storage/spin.h"

  int            DeadlockTimeout = 1000;
+ int            QueryTimeout = 0;
+ int            RemainingQueryTimeout = 0;
+ bool        alarm_is_query_timeout = false;

  PGPROC       *MyProc = NULL;

***************
*** 319,325 ****
      waitingForLock = false;

      /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
!     disable_sigalrm_interrupt();

      /* Unlink myself from the wait queue, if on it (might not be anymore!) */
      LWLockAcquire(LockMgrLock, LW_EXCLUSIVE);
--- 321,327 ----
      waitingForLock = false;

      /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
!     disable_sig_alarm(false);

      /* Unlink myself from the wait queue, if on it (might not be anymore!) */
      LWLockAcquire(LockMgrLock, LW_EXCLUSIVE);
***************
*** 632,638 ****
       * By delaying the check until we've waited for a bit, we can avoid
       * running the rather expensive deadlock-check code in most cases.
       */
!     if (!enable_sigalrm_interrupt(DeadlockTimeout))
          elog(FATAL, "ProcSleep: Unable to set timer for process wakeup");

      /*
--- 634,640 ----
       * By delaying the check until we've waited for a bit, we can avoid
       * running the rather expensive deadlock-check code in most cases.
       */
!     if (!enable_sig_alarm(DeadlockTimeout, false))
          elog(FATAL, "ProcSleep: Unable to set timer for process wakeup");

      /*
***************
*** 654,660 ****
      /*
       * Disable the timer, if it's still running
       */
!     if (!disable_sigalrm_interrupt())
          elog(FATAL, "ProcSleep: Unable to disable timer for process wakeup");

      /*
--- 656,662 ----
      /*
       * Disable the timer, if it's still running
       */
!     if (!disable_sig_alarm(false))
          elog(FATAL, "ProcSleep: Unable to disable timer for process wakeup");

      /*
***************
*** 785,791 ****
   * --------------------
   */
  void
! HandleDeadLock(SIGNAL_ARGS)
  {
      int            save_errno = errno;

--- 787,793 ----
   * --------------------
   */
  void
! HandleDeadLock(void)
  {
      int            save_errno = errno;

***************
*** 921,949 ****
   * Delay is given in milliseconds.    Caller should be sure a SIGALRM
   * signal handler is installed before this is called.
   *
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! enable_sigalrm_interrupt(int delayms)
  {
  #ifndef __BEOS__
!     struct itimerval timeval,
!                 dummy;

      MemSet(&timeval, 0, sizeof(struct itimerval));
      timeval.it_value.tv_sec = delayms / 1000;
      timeval.it_value.tv_usec = (delayms % 1000) * 1000;
!     if (setitimer(ITIMER_REAL, &timeval, &dummy))
          return false;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
-     bigtime_t    time_interval;
-
      time_interval = delayms * 1000;        /* usecs */
!     if (set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM) < 0)
          return false;
  #endif

      return true;
  }

--- 923,1014 ----
   * Delay is given in milliseconds.    Caller should be sure a SIGALRM
   * signal handler is installed before this is called.
   *
+  * This code properly handles multiple alarms when when the query_timeout
+  * alarm is specified first.
+  *
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! enable_sig_alarm(int delayms, bool is_query_timeout)
  {
  #ifndef __BEOS__
!     struct itimerval timeval, remaining;
! #else
!     bigtime_t    time_interval, remaining;
! #endif
!     int remainingms;

+     /* Don't set timer if the query timeout scheduled before next alarm. */
+     if (alarm_is_query_timeout &&
+         !is_query_timeout &&
+         RemainingQueryTimeout <= delayms)
+         return true;
+
+ #ifndef __BEOS__
      MemSet(&timeval, 0, sizeof(struct itimerval));
      timeval.it_value.tv_sec = delayms / 1000;
      timeval.it_value.tv_usec = (delayms % 1000) * 1000;
!     if (setitimer(ITIMER_REAL, &timeval, &remaining))
          return false;
+     if (alarm_is_query_timeout && !is_query_timeout)
+     {
+         remainingms = remaining.it_value.tv_sec * 1000 +
+                       remaining.it_value.tv_usec;
+         /* Query already timed out */
+         if (remainingms == 0)
+         {
+             alarm_is_query_timeout = true;
+             kill(MyProcPid, SIGALRM);
+         }
+         /* Previous alarm < delayms? */
+         if (remainingms < delayms)
+         {
+             alarm_is_query_timeout = true;
+             /* return alarm as though no change was made */
+              if (setitimer(ITIMER_REAL, &remaining, &timeval))
+                 return false;
+             else
+                 return true;
+         }
+         RemainingQueryTimeout = remainingms - delayms;
+     }
+     else if (is_query_timeout)
+         RemainingQueryTimeout = QueryTimeout;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
      time_interval = delayms * 1000;        /* usecs */
!     if ((remaining = set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM)) < 0)
          return false;
+     if (alarm_is_query_timeout && !is_query_timeout)
+     {
+         remainingms = remaining / 1000;
+         /* Query already timed out */
+         if (remainingms == 0)
+         {
+             alarm_is_query_timeout = true;
+             kill(MyProcPid, SIGALRM);
+         }
+         /* Previous alarm < delayms? */
+         if (remainingms < delayms)
+         {
+             alarm_is_query_timeout = true;
+             /* return as though no change was made */
+             if ((timeval = set_alarm(remaining, B_ONE_SHOT_RELATIVE_ALARM)) < 0)
+                 return false;
+             else
+                 return true;
+         }
+         RemainingQueryTimeout = remainingms - delayms;
+     }
+     else if (is_query_timeout)
+         RemainingQueryTimeout = QueryTimeout;
  #endif

+     if (is_query_timeout)
+         alarm_is_query_timeout = true;
+     else
+         alarm_is_query_timeout = false;
+
      return true;
  }

***************
*** 953,972 ****
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! disable_sigalrm_interrupt(void)
  {
  #ifndef __BEOS__
!     struct itimerval timeval,
!                 dummy;

      MemSet(&timeval, 0, sizeof(struct itimerval));
!     if (setitimer(ITIMER_REAL, &timeval, &dummy))
!         return false;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
!     if (set_alarm(B_INFINITE_TIMEOUT, B_PERIODIC_ALARM) < 0)
!         return false;
  #endif

      return true;
  }
--- 1018,1087 ----
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! disable_sig_alarm(bool is_query_timeout)
  {
  #ifndef __BEOS__
!     struct itimerval timeval, dummy;
! #endif

+ #ifndef __BEOS__
      MemSet(&timeval, 0, sizeof(struct itimerval));
!     if (!is_query_timeout && RemainingQueryTimeout)
!     {
!         /* Restore remaining query timeout value */
!         timeval.it_value.tv_sec = RemainingQueryTimeout / 1000;
!         timeval.it_value.tv_usec = (RemainingQueryTimeout % 1000) * 1000;
!         alarm_is_query_timeout = true;
!     }
!     /*
!      *    Optimization: is_query_timeout && RemainingQueryTimeout == 0
!      *  does nothing.  This is for cases where no timeout was set.
!      */
!     if (!is_query_timeout || RemainingQueryTimeout)
!     {
!         if (setitimer(ITIMER_REAL, &timeval, &dummy))
!             return false;
!     }
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
!     if (!is_query_timeout && RemainingQueryTimeout)
!     {
!         bigtime_t    time_interval = RemainingQueryTimeout * 1000;
!
!         alarm_is_query_timeout = true;
!         if (!is_query_timeout)
!         {
!             if (set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM) < 0)
!                 return false;
!         }
!     }
!     else if (!is_query_timeout || RemainingQueryTimeout)
!     {
!         if (set_alarm(B_INFINITE_TIMEOUT, B_PERIODIC_ALARM) < 0)
!             return false;
!     }
  #endif

+     if (is_query_timeout)
+         RemainingQueryTimeout = 0;
+
      return true;
  }
+
+
+ /*
+  * Call alarm handler.
+  */
+ void
+ handle_sig_alarm(SIGNAL_ARGS)
+ {
+     if (alarm_is_query_timeout)
+     {
+         RemainingQueryTimeout = 0;
+         alarm_is_query_timeout = false;
+         kill(MyProcPid, SIGINT);
+     }
+     else
+         HandleDeadLock();
+ }
+
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.268
diff -c -r1.268 postgres.c
*** src/backend/tcop/postgres.c    20 Jun 2002 20:29:36 -0000    1.268
--- src/backend/tcop/postgres.c    10 Jul 2002 03:58:58 -0000
***************
*** 78,83 ****
--- 78,85 ----
  /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */
  CommandDest whereToSendOutput = Debug;

+ extern int QueryTimeout;
+
  static bool dontExecute = false;

  /* note: these declarations had better match tcopprot.h */
***************
*** 723,728 ****
--- 725,733 ----
               */
              CHECK_FOR_INTERRUPTS();

+             if (QueryTimeout)
+                 enable_sig_alarm(QueryTimeout, true);
+
              if (querytree->commandType == CMD_UTILITY)
              {
                  /*
***************
*** 791,796 ****
--- 796,803 ----
                      ShowUsage("EXECUTOR STATISTICS");
              }

+             disable_sig_alarm(true);
+
              /*
               * In a query block, we want to increment the command counter
               * between queries so that the effects of early queries are
***************
*** 821,829 ****
                  finish_xact_command();
                  xact_started = false;
              }
!
!         }                        /* end loop over queries generated from a
!                                  * parsetree */

          /*
           * If this is the last parsetree of the query string, close down
--- 828,834 ----
                  finish_xact_command();
                  xact_started = false;
              }
!         } /* end loop over queries generated from a parsetree */

          /*
           * If this is the last parsetree of the query string, close down
***************
*** 1554,1560 ****
      pqsignal(SIGINT, QueryCancelHandler);        /* cancel current query */
      pqsignal(SIGTERM, die);        /* cancel current query and exit */
      pqsignal(SIGQUIT, quickdie);    /* hard crash time */
!     pqsignal(SIGALRM, HandleDeadLock);    /* check for deadlock after
                                           * timeout */

      /*
--- 1559,1565 ----
      pqsignal(SIGINT, QueryCancelHandler);        /* cancel current query */
      pqsignal(SIGTERM, die);        /* cancel current query and exit */
      pqsignal(SIGQUIT, quickdie);    /* hard crash time */
!     pqsignal(SIGALRM, handle_sig_alarm);    /* check for deadlock after
                                           * timeout */

      /*
***************
*** 1819,1824 ****
--- 1824,1832 ----
           */
          QueryCancelPending = false;        /* forget any earlier CANCEL
                                           * signal */
+
+         /* Stop any query timer */
+         disable_sig_alarm(true);

          EnableNotifyInterrupt();

Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.70
diff -c -r1.70 guc.c
*** src/backend/utils/misc/guc.c    16 Jun 2002 00:09:12 -0000    1.70
--- src/backend/utils/misc/guc.c    10 Jul 2002 03:59:00 -0000
***************
*** 51,56 ****
--- 51,57 ----
  extern bool Log_connections;
  extern int    PreAuthDelay;
  extern int    AuthenticationTimeout;
+ extern int    QueryTimeout;
  extern int    CheckPointTimeout;
  extern int    CommitDelay;
  extern int    CommitSiblings;
***************
*** 573,578 ****
--- 574,584 ----
      {
          { "max_expr_depth", PGC_USERSET }, &max_expr_depth,
          DEFAULT_MAX_EXPR_DEPTH, 10, INT_MAX, NULL, NULL
+     },
+
+     {
+         { "query_timeout", PGC_USERSET }, &QueryTimeout,
+         0, 0, INT_MAX, NULL, NULL
      },

      {
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.40
diff -c -r1.40 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample    16 Jun 2002 00:09:12 -0000    1.40
--- src/backend/utils/misc/postgresql.conf.sample    10 Jul 2002 03:59:00 -0000
***************
*** 200,202 ****
--- 200,203 ----
  #password_encryption = true
  #sql_inheritance = true
  #transform_null_equals = false
+ #query_timeout = 0                # 0 is disabled
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.50
diff -c -r1.50 tab-complete.c
*** src/bin/psql/tab-complete.c    16 Jun 2002 00:09:12 -0000    1.50
--- src/bin/psql/tab-complete.c    10 Jul 2002 03:59:05 -0000
***************
*** 267,272 ****
--- 267,273 ----

          "default_transaction_isolation",
          "search_path",
+         "query_timeout",

          NULL
      };
Index: src/include/storage/proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/storage/proc.h,v
retrieving revision 1.57
diff -c -r1.57 proc.h
*** src/include/storage/proc.h    20 Jun 2002 20:29:52 -0000    1.57
--- src/include/storage/proc.h    10 Jul 2002 03:59:05 -0000
***************
*** 105,117 ****
  extern PGPROC *ProcWakeup(PGPROC *proc, int errType);
  extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock);
  extern bool LockWaitCancel(void);
! extern void HandleDeadLock(SIGNAL_ARGS);

  extern void ProcWaitForSignal(void);
  extern void ProcCancelWaitForSignal(void);
  extern void ProcSendSignal(BackendId procId);

! extern bool enable_sigalrm_interrupt(int delayms);
! extern bool disable_sigalrm_interrupt(void);

  #endif   /* PROC_H */
--- 105,118 ----
  extern PGPROC *ProcWakeup(PGPROC *proc, int errType);
  extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock);
  extern bool LockWaitCancel(void);
! extern void HandleDeadLock(void);

  extern void ProcWaitForSignal(void);
  extern void ProcCancelWaitForSignal(void);
  extern void ProcSendSignal(BackendId procId);

! extern bool enable_sig_alarm(int delayms, bool is_query_timeout);
! extern bool disable_sig_alarm(bool is_query_timeout);
! extern void handle_sig_alarm(SIGNAL_ARGS);

  #endif   /* PROC_H */

Bruce Momjian writes:

> Here is my first draft of a query timeout SET variable.

Unless it only cancels SELECT statements (which may or may not be a good
idea), please call it statement_timeout.

--
Peter Eisentraut   peter_e@gmx.net

Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > Here is my first draft of a query timeout SET variable.
>
> Unless it only cancels SELECT statements (which may or may not be a good
> idea), please call it statement_timeout.

Woh, you mean only SELECT is a query?  Why is it SQL?  Do we not use
'query' to mean any SQL command?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Peter Eisentraut wrote:
> Bruce Momjian writes:
>
> > Here is my first draft of a query timeout SET variable.
>
> Unless it only cancels SELECT statements (which may or may not be a good
> idea), please call it statement_timeout.

Do people prefer query_timeout or statement_timeout?  Doesn't matter to
me.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

On Wed, 2002-07-10 at 18:52, Bruce Momjian wrote:
> Peter Eisentraut wrote:
> > Bruce Momjian writes:
> >
> > > Here is my first draft of a query timeout SET variable.
> >
> > Unless it only cancels SELECT statements (which may or may not be a good
> > idea), please call it statement_timeout.
>
> Do people prefer query_timeout or statement_timeout?  Doesn't matter to
> me.

There is no 'statement' in SQL, that said, 75% of SQL has nothing to do
with queries.

So... I think I vote 'statement'.

Bruce Momjian wrote:
>
> Peter Eisentraut wrote:
> > Bruce Momjian writes:
> >
> > > Here is my first draft of a query timeout SET variable.
> >
> > Unless it only cancels SELECT statements (which may or may not be a good
> > idea), please call it statement_timeout.
>
> Do people prefer query_timeout or statement_timeout?  Doesn't matter to
> me.

Statements is everything. DDL- and DML-statements. Query is IMHO synonym
for DML-statement. So query_timeout is the right term.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

Jan Wieck wrote:
> Bruce Momjian wrote:
> >
> > Peter Eisentraut wrote:
> > > Bruce Momjian writes:
> > >
> > > > Here is my first draft of a query timeout SET variable.
> > >
> > > Unless it only cancels SELECT statements (which may or may not be a good
> > > idea), please call it statement_timeout.
> >
> > Do people prefer query_timeout or statement_timeout?  Doesn't matter to
> > me.
>
> Statements is everything. DDL- and DML-statements. Query is IMHO synonym
> for DML-statement. So query_timeout is the right term.

But the timeout is for any statement, not just SELECT/UPDATE, etc, so it
sounds like you are voting for 'statement'.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

The INFORMIX equivalent is SET LOCK MODE TO WAIT [timeout value in seconds];
If you don't specify any timeout value, you wait until all statements are
completed, If timeout value is set to 0 then you return immediatly if the
tables you query are already locked, ...

This statement is valid for a connection or until another identical
statement is sent.
----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: "Jan Wieck" <JanWieck@Yahoo.com>
Cc: "Peter Eisentraut" <peter_e@gmx.net>; "Ed Loehr" <ed@LoehrTech.com>;
<dave@fastcrypt.com>; "Matthew Kennedy" <mkennedy@opushealthcare.com>;
"PostgreSQL-patches" <pgsql-patches@postgresql.org>
Sent: Thursday, July 11, 2002 11:34 AM
Subject: Re: [PATCHES] implementing query timeout


> Jan Wieck wrote:
> > Bruce Momjian wrote:
> > >
> > > Peter Eisentraut wrote:
> > > > Bruce Momjian writes:
> > > >
> > > > > Here is my first draft of a query timeout SET variable.
> > > >
> > > > Unless it only cancels SELECT statements (which may or may not be a
good
> > > > idea), please call it statement_timeout.
> > >
> > > Do people prefer query_timeout or statement_timeout?  Doesn't matter
to
> > > me.
> >
> > Statements is everything. DDL- and DML-statements. Query is IMHO synonym
> > for DML-statement. So query_timeout is the right term.
>
> But the timeout is for any statement, not just SELECT/UPDATE, etc, so it
> sounds like you are voting for 'statement'.
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
>
>

Nicolas Bazin wrote:
> The INFORMIX equivalent is SET LOCK MODE TO WAIT [timeout value in seconds];
> If you don't specify any timeout value, you wait until all statements are
> completed, If timeout value is set to 0 then you return immediatly if the
> tables you query are already locked, ...
>
> This statement is valid for a connection or until another identical
> statement is sent.

Users want a more general timeout facitily, for example, queries taking
 > 10 minutes.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian wrote:
>
> Jan Wieck wrote:

> > Statements is everything. DDL- and DML-statements. Query is IMHO synonym
> > for DML-statement. So query_timeout is the right term.
>
> But the timeout is for any statement, not just SELECT/UPDATE, etc, so it
> sounds like you are voting for 'statement'.

No, I am voting for 'query'. I don't see the point in allowing a
timeout for utility statements. Why would someone want a timeout
on CREATE INDEX, COPY or VACUUM? Allowing that would IMHO be
calling for more trouble than necessary.


Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being
right. #
# Let's break this rule - forgive
me.                                  #
#==================================================
JanWieck@Yahoo.com #

Jan Wieck wrote:
> Bruce Momjian wrote:
> >
> > Jan Wieck wrote:
>
> > > Statements is everything. DDL- and DML-statements. Query is IMHO synonym
> > > for DML-statement. So query_timeout is the right term.
> >
> > But the timeout is for any statement, not just SELECT/UPDATE, etc, so it
> > sounds like you are voting for 'statement'.
>
> No, I am voting for 'query'. I don't see the point in allowing a
> timeout for utility statements. Why would someone want a timeout
> on CREATE INDEX, COPY or VACUUM? Allowing that would IMHO be
> calling for more trouble than necessary.

Seems pretty arbitrary to time just DML and not DLL.  I can even imagine
this for VACUUM FULL where you don't want it running for a long time.
It is under their control and they can turn it off if they don't want it
for those statements.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Rod Taylor wrote:
> On Wed, 2002-07-10 at 18:52, Bruce Momjian wrote:
> > Peter Eisentraut wrote:
> > > Bruce Momjian writes:
> > >
> > > > Here is my first draft of a query timeout SET variable.
> > >
> > > Unless it only cancels SELECT statements (which may or may not be a good
> > > idea), please call it statement_timeout.
> >
> > Do people prefer query_timeout or statement_timeout?  Doesn't matter to
> > me.
>
> There is no 'statement' in SQL, that said, 75% of SQL has nothing to do
> with queries.
>
> So... I think I vote 'statement'.

OK, changed to 'statement'.  Patch completed, ready for testing.  It
properly handles all the cases I tested, like UPDATE waiting on a lock
and SELECT queries.

I also tested setting statement_timeout to '1' and it allows you to
change it to a different value.  It doesn't cancel the SET before it is
changed.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/runtime.sgml,v
retrieving revision 1.120
diff -c -r1.120 runtime.sgml
*** doc/src/sgml/runtime.sgml    5 Jul 2002 01:17:20 -0000    1.120
--- doc/src/sgml/runtime.sgml    11 Jul 2002 19:08:18 -0000
***************
*** 1585,1590 ****
--- 1585,1600 ----
       </varlistentry>

       <varlistentry>
+       <term><varname>STATEMENT_TIMEOUT</varname> (<type>integer</type>)</term>
+       <listitem>
+        <para>
+         Aborts any statement that takes over the specified number of
+         microseconds.  A value of zero turns off the timer.
+        </para>
+       </listitem>
+      </varlistentry>
+
+      <varlistentry>
        <term><varname>SHARED_BUFFERS</varname> (<type>integer</type>)</term>
        <listitem>
         <para>
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/postmaster/postmaster.c,v
retrieving revision 1.280
diff -c -r1.280 postmaster.c
*** src/backend/postmaster/postmaster.c    20 Jun 2002 20:29:33 -0000    1.280
--- src/backend/postmaster/postmaster.c    11 Jul 2002 19:08:20 -0000
***************
*** 2105,2111 ****
       * after a time delay, so that a broken client can't hog a connection
       * indefinitely.  PreAuthDelay doesn't count against the time limit.
       */
!     if (!enable_sigalrm_interrupt(AuthenticationTimeout * 1000))
          elog(FATAL, "DoBackend: Unable to set timer for auth timeout");

      /*
--- 2105,2111 ----
       * after a time delay, so that a broken client can't hog a connection
       * indefinitely.  PreAuthDelay doesn't count against the time limit.
       */
!     if (!enable_sig_alarm(AuthenticationTimeout * 1000, false))
          elog(FATAL, "DoBackend: Unable to set timer for auth timeout");

      /*
***************
*** 2134,2140 ****
       * Done with authentication.  Disable timeout, and prevent
       * SIGTERM/SIGQUIT again until backend startup is complete.
       */
!     if (!disable_sigalrm_interrupt())
          elog(FATAL, "DoBackend: Unable to disable timer for auth timeout");
      PG_SETMASK(&BlockSig);

--- 2134,2140 ----
       * Done with authentication.  Disable timeout, and prevent
       * SIGTERM/SIGQUIT again until backend startup is complete.
       */
!     if (!disable_sig_alarm(false))
          elog(FATAL, "DoBackend: Unable to disable timer for auth timeout");
      PG_SETMASK(&BlockSig);

Index: src/backend/storage/lmgr/proc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/lmgr/proc.c,v
retrieving revision 1.121
diff -c -r1.121 proc.c
*** src/backend/storage/lmgr/proc.c    20 Jun 2002 20:29:35 -0000    1.121
--- src/backend/storage/lmgr/proc.c    11 Jul 2002 19:08:21 -0000
***************
*** 52,59 ****
  #include "storage/sinval.h"
  #include "storage/spin.h"

-
  int            DeadlockTimeout = 1000;

  PGPROC       *MyProc = NULL;

--- 52,61 ----
  #include "storage/sinval.h"
  #include "storage/spin.h"

  int            DeadlockTimeout = 1000;
+ int            StatementTimeout = 0;
+ int            RemainingStatementTimeout = 0;
+ bool        alarm_is_statement_timeout = false;

  PGPROC       *MyProc = NULL;

***************
*** 319,325 ****
      waitingForLock = false;

      /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
!     disable_sigalrm_interrupt();

      /* Unlink myself from the wait queue, if on it (might not be anymore!) */
      LWLockAcquire(LockMgrLock, LW_EXCLUSIVE);
--- 321,327 ----
      waitingForLock = false;

      /* Turn off the deadlock timer, if it's still running (see ProcSleep) */
!     disable_sig_alarm(false);

      /* Unlink myself from the wait queue, if on it (might not be anymore!) */
      LWLockAcquire(LockMgrLock, LW_EXCLUSIVE);
***************
*** 632,638 ****
       * By delaying the check until we've waited for a bit, we can avoid
       * running the rather expensive deadlock-check code in most cases.
       */
!     if (!enable_sigalrm_interrupt(DeadlockTimeout))
          elog(FATAL, "ProcSleep: Unable to set timer for process wakeup");

      /*
--- 634,640 ----
       * By delaying the check until we've waited for a bit, we can avoid
       * running the rather expensive deadlock-check code in most cases.
       */
!     if (!enable_sig_alarm(DeadlockTimeout, false))
          elog(FATAL, "ProcSleep: Unable to set timer for process wakeup");

      /*
***************
*** 654,660 ****
      /*
       * Disable the timer, if it's still running
       */
!     if (!disable_sigalrm_interrupt())
          elog(FATAL, "ProcSleep: Unable to disable timer for process wakeup");

      /*
--- 656,662 ----
      /*
       * Disable the timer, if it's still running
       */
!     if (!disable_sig_alarm(false))
          elog(FATAL, "ProcSleep: Unable to disable timer for process wakeup");

      /*
***************
*** 785,791 ****
   * --------------------
   */
  void
! HandleDeadLock(SIGNAL_ARGS)
  {
      int            save_errno = errno;

--- 787,793 ----
   * --------------------
   */
  void
! HandleDeadLock(void)
  {
      int            save_errno = errno;

***************
*** 921,949 ****
   * Delay is given in milliseconds.    Caller should be sure a SIGALRM
   * signal handler is installed before this is called.
   *
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! enable_sigalrm_interrupt(int delayms)
  {
  #ifndef __BEOS__
!     struct itimerval timeval,
!                 dummy;

      MemSet(&timeval, 0, sizeof(struct itimerval));
      timeval.it_value.tv_sec = delayms / 1000;
      timeval.it_value.tv_usec = (delayms % 1000) * 1000;
!     if (setitimer(ITIMER_REAL, &timeval, &dummy))
          return false;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
-     bigtime_t    time_interval;
-
      time_interval = delayms * 1000;        /* usecs */
!     if (set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM) < 0)
          return false;
  #endif

      return true;
  }

--- 923,1012 ----
   * Delay is given in milliseconds.    Caller should be sure a SIGALRM
   * signal handler is installed before this is called.
   *
+  * This code properly handles multiple alarms when when the statement_timeout
+  * alarm is specified first.
+  *
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! enable_sig_alarm(int delayms, bool is_statement_timeout)
  {
  #ifndef __BEOS__
!     struct itimerval timeval, remaining;
! #else
!     bigtime_t    time_interval, remaining;
! #endif

+     /* Don't set timer if the statement timeout scheduled before next alarm. */
+     if (alarm_is_statement_timeout &&
+         !is_statement_timeout &&
+         RemainingStatementTimeout <= delayms)
+         return true;
+
+ #ifndef __BEOS__
      MemSet(&timeval, 0, sizeof(struct itimerval));
      timeval.it_value.tv_sec = delayms / 1000;
      timeval.it_value.tv_usec = (delayms % 1000) * 1000;
!     if (setitimer(ITIMER_REAL, &timeval, &remaining))
          return false;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
      time_interval = delayms * 1000;        /* usecs */
!     if ((remaining = set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM)) < 0)
          return false;
  #endif

+     if (is_statement_timeout)
+         RemainingStatementTimeout = StatementTimeout;
+     else
+     {
+         /* Switching to non-statement-timeout alarm, get remaining time */
+         if (alarm_is_statement_timeout)
+         {
+ #ifndef __BEOS__
+             /* We lose precision here because we convert to milliseconds */
+             RemainingStatementTimeout = remaining.it_value.tv_sec * 1000 +
+                                         remaining.it_value.tv_usec / 1000;
+ #else
+             RemainingStatementTimeout = remaining / 1000;
+ #endif
+             /* Rounding could cause a zero */
+             if (RemainingStatementTimeout == 0)
+                 RemainingStatementTimeout = 1;
+         }
+
+         if (RemainingStatementTimeout)
+         {
+             /* Remaining timeout alarm < delayms? */
+             if (RemainingStatementTimeout <= delayms)
+             {
+                 /* reinstall statement timeout alarm */
+                 alarm_is_statement_timeout = true;
+ #ifndef __BEOS__
+                 remaining.it_value.tv_sec = RemainingStatementTimeout / 1000;
+                 remaining.it_value.tv_usec = (RemainingStatementTimeout % 1000) * 1000;
+                  if (setitimer(ITIMER_REAL, &remaining, &timeval))
+                     return false;
+                 else
+                     return true;
+ #else
+                 remaining = RemainingStatementTimeout * 1000;
+                 if ((timeval = set_alarm(remaining, B_ONE_SHOT_RELATIVE_ALARM)) < 0)
+                     return false;
+                 else
+                     return true;
+ #endif
+             }
+             else
+                 RemainingStatementTimeout -= delayms;
+         }
+     }
+
+     if (is_statement_timeout)
+         alarm_is_statement_timeout = true;
+     else
+         alarm_is_statement_timeout = false;
+
      return true;
  }

***************
*** 953,972 ****
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! disable_sigalrm_interrupt(void)
  {
  #ifndef __BEOS__
!     struct itimerval timeval,
!                 dummy;

      MemSet(&timeval, 0, sizeof(struct itimerval));
!     if (setitimer(ITIMER_REAL, &timeval, &dummy))
!         return false;
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
!     if (set_alarm(B_INFINITE_TIMEOUT, B_PERIODIC_ALARM) < 0)
!         return false;
  #endif

      return true;
  }
--- 1016,1087 ----
   * Returns TRUE if okay, FALSE on failure.
   */
  bool
! disable_sig_alarm(bool is_statement_timeout)
  {
  #ifndef __BEOS__
!     struct itimerval timeval, dummy;

      MemSet(&timeval, 0, sizeof(struct itimerval));
!     if (!is_statement_timeout && RemainingStatementTimeout)
!     {
!         /* Restore remaining statement timeout value */
!         alarm_is_statement_timeout = true;
!         timeval.it_value.tv_sec = RemainingStatementTimeout / 1000;
!         timeval.it_value.tv_usec = (RemainingStatementTimeout % 1000) * 1000;
!     }
!     /*
!      *    Optimization: is_statement_timeout && RemainingStatementTimeout == 0
!      *  does nothing.  This is for cases where no timeout was set.
!      */
!     if (!is_statement_timeout || RemainingStatementTimeout)
!     {
!         if (setitimer(ITIMER_REAL, &timeval, &dummy))
!             return false;
!     }
  #else
      /* BeOS doesn't have setitimer, but has set_alarm */
!     if (!is_statement_timeout && RemainingStatementTimeout)
!     {
!         bigtime_t    time_interval = RemainingStatementTimeout * 1000;
!
!         alarm_is_statement_timeout = true;
!         if (!is_statement_timeout)
!         {
!             if (set_alarm(time_interval, B_ONE_SHOT_RELATIVE_ALARM) < 0)
!                 return false;
!         }
!     }
!     else if (!is_statement_timeout || RemainingStatementTimeout)
!     {
!         if (set_alarm(B_INFINITE_TIMEOUT, B_PERIODIC_ALARM) < 0)
!             return false;
!     }
  #endif

+     if (is_statement_timeout)
+         RemainingStatementTimeout = 0;
+
      return true;
  }
+
+
+ /*
+  * Call alarm handler.
+  */
+ void
+ handle_sig_alarm(SIGNAL_ARGS)
+ {
+     if (alarm_is_statement_timeout)
+     {
+         RemainingStatementTimeout = 0;
+         alarm_is_statement_timeout = false;
+         kill(MyProcPid, SIGINT);
+     }
+     else
+     {
+         HandleDeadLock();
+         /* Reactivate any statement_timeout alarm. */
+         disable_sig_alarm(false);
+     }
+ }
+
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.268
diff -c -r1.268 postgres.c
*** src/backend/tcop/postgres.c    20 Jun 2002 20:29:36 -0000    1.268
--- src/backend/tcop/postgres.c    11 Jul 2002 19:08:24 -0000
***************
*** 78,83 ****
--- 78,85 ----
  /* Note: whereToSendOutput is initialized for the bootstrap/standalone case */
  CommandDest whereToSendOutput = Debug;

+ extern int StatementTimeout;
+
  static bool dontExecute = false;

  /* note: these declarations had better match tcopprot.h */
***************
*** 717,722 ****
--- 719,727 ----
                  xact_started = true;
              }

+             if (StatementTimeout)
+                 enable_sig_alarm(StatementTimeout, true);
+
              /*
               * If we got a cancel signal in analysis or prior command,
               * quit
***************
*** 791,796 ****
--- 796,803 ----
                      ShowUsage("EXECUTOR STATISTICS");
              }

+             disable_sig_alarm(true);
+
              /*
               * In a query block, we want to increment the command counter
               * between queries so that the effects of early queries are
***************
*** 821,829 ****
                  finish_xact_command();
                  xact_started = false;
              }
!
!         }                        /* end loop over queries generated from a
!                                  * parsetree */

          /*
           * If this is the last parsetree of the query string, close down
--- 828,834 ----
                  finish_xact_command();
                  xact_started = false;
              }
!         } /* end loop over queries generated from a parsetree */

          /*
           * If this is the last parsetree of the query string, close down
***************
*** 996,1002 ****
   * at soonest convenient time
   */
  static void
! QueryCancelHandler(SIGNAL_ARGS)
  {
      int            save_errno = errno;

--- 1001,1007 ----
   * at soonest convenient time
   */
  static void
! StatementCancelHandler(SIGNAL_ARGS)
  {
      int            save_errno = errno;

***************
*** 1551,1560 ****
       */

      pqsignal(SIGHUP, SigHupHandler);    /* set flag to read config file */
!     pqsignal(SIGINT, QueryCancelHandler);        /* cancel current query */
      pqsignal(SIGTERM, die);        /* cancel current query and exit */
      pqsignal(SIGQUIT, quickdie);    /* hard crash time */
!     pqsignal(SIGALRM, HandleDeadLock);    /* check for deadlock after
                                           * timeout */

      /*
--- 1556,1565 ----
       */

      pqsignal(SIGHUP, SigHupHandler);    /* set flag to read config file */
!     pqsignal(SIGINT, StatementCancelHandler);        /* cancel current query */
      pqsignal(SIGTERM, die);        /* cancel current query and exit */
      pqsignal(SIGQUIT, quickdie);    /* hard crash time */
!     pqsignal(SIGALRM, handle_sig_alarm);    /* check for deadlock after
                                           * timeout */

      /*
***************
*** 1819,1824 ****
--- 1824,1832 ----
           */
          QueryCancelPending = false;        /* forget any earlier CANCEL
                                           * signal */
+
+         /* Stop any statement timer */
+         disable_sig_alarm(true);

          EnableNotifyInterrupt();

Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.70
diff -c -r1.70 guc.c
*** src/backend/utils/misc/guc.c    16 Jun 2002 00:09:12 -0000    1.70
--- src/backend/utils/misc/guc.c    11 Jul 2002 19:08:25 -0000
***************
*** 51,56 ****
--- 51,57 ----
  extern bool Log_connections;
  extern int    PreAuthDelay;
  extern int    AuthenticationTimeout;
+ extern int    StatementTimeout;
  extern int    CheckPointTimeout;
  extern int    CommitDelay;
  extern int    CommitSiblings;
***************
*** 573,578 ****
--- 574,584 ----
      {
          { "max_expr_depth", PGC_USERSET }, &max_expr_depth,
          DEFAULT_MAX_EXPR_DEPTH, 10, INT_MAX, NULL, NULL
+     },
+
+     {
+         { "statement_timeout", PGC_USERSET }, &StatementTimeout,
+         0, 0, INT_MAX, NULL, NULL
      },

      {
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.40
diff -c -r1.40 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample    16 Jun 2002 00:09:12 -0000    1.40
--- src/backend/utils/misc/postgresql.conf.sample    11 Jul 2002 19:08:25 -0000
***************
*** 200,202 ****
--- 200,203 ----
  #password_encryption = true
  #sql_inheritance = true
  #transform_null_equals = false
+ #statement_timeout = 0                # 0 is disabled
Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.50
diff -c -r1.50 tab-complete.c
*** src/bin/psql/tab-complete.c    16 Jun 2002 00:09:12 -0000    1.50
--- src/bin/psql/tab-complete.c    11 Jul 2002 19:08:31 -0000
***************
*** 267,272 ****
--- 267,273 ----

          "default_transaction_isolation",
          "search_path",
+         "statement_timeout",

          NULL
      };
Index: src/include/storage/proc.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/storage/proc.h,v
retrieving revision 1.57
diff -c -r1.57 proc.h
*** src/include/storage/proc.h    20 Jun 2002 20:29:52 -0000    1.57
--- src/include/storage/proc.h    11 Jul 2002 19:08:31 -0000
***************
*** 105,117 ****
  extern PGPROC *ProcWakeup(PGPROC *proc, int errType);
  extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock);
  extern bool LockWaitCancel(void);
! extern void HandleDeadLock(SIGNAL_ARGS);

  extern void ProcWaitForSignal(void);
  extern void ProcCancelWaitForSignal(void);
  extern void ProcSendSignal(BackendId procId);

! extern bool enable_sigalrm_interrupt(int delayms);
! extern bool disable_sigalrm_interrupt(void);

  #endif   /* PROC_H */
--- 105,118 ----
  extern PGPROC *ProcWakeup(PGPROC *proc, int errType);
  extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock);
  extern bool LockWaitCancel(void);
! extern void HandleDeadLock(void);

  extern void ProcWaitForSignal(void);
  extern void ProcCancelWaitForSignal(void);
  extern void ProcSendSignal(BackendId procId);

! extern bool enable_sig_alarm(int delayms, bool is_statement_timeout);
! extern bool disable_sig_alarm(bool is_statement_timeout);
! extern void handle_sig_alarm(SIGNAL_ARGS);

  #endif   /* PROC_H */