Re: Statement timeout behavior in extended queries

From: pgsql-hackers-owner(at)postgresql(dot)org
> [mailto:pgsql-hackers-owner(at)postgresql(dot)org] On Behalf Of Tatsuo Ishii
> No. Parse, bind and Execute message indivually stops and starts the
> statement_timeout timer with the patch. Unless there's a lock conflict,
> parse and bind will take very short time. So actually users could only
> observe the timeout in execute message though.

Where is the statement_timeout timer stopped when processing Parse and Bind messages? Do you mean the following sequence of operations are performed in this patch?

Parse(statement1)
start timer
stop timer
Bind(statement1, portal1)
start timer
stop timer
Execute(portal1)
start timer
stop timer
Sync

It looks like the patch does the following. I think this is desirable, because starting and stopping the timer for each message may be costly as Tom said.

Parse(statement1)
start timer
Bind(statement1, portal1)
Execute(portal1)
stop timer
Sync

> > (3)
> > + /*
> > + * Sanity check
> > + */
> > + if (!xact_started)
> > + {
> > + ereport(ERROR,
> > +
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> > + errmsg("Transaction
> must have been already started to set statement timeout")));
> > + }
> >
> > I think this fragment can be deleted, because enable/disable_timeout()
> is used only at limited places in postgres.c, so I don't see the chance
> of misuse.
>
> I'd suggest leave it as it is. Because it might be possible that the function
> is used in different place in the future. Or at least we should document
> the pre-condition as a comment.

OK, I favor documenting to reduce code, but I don't have a strong opinion. I'd like to leave this to the committer. One thing to note is that you can remove { and } in the following code like other places.

+ if (!xact_started)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("Transaction must have been already started to set statement timeout")));
+ }

Regards
Takayuki Tsunakawa