Re: performance bug in instrument.c

Tomas Vondra <tv(at)fuzzy(dot)cz> writes:
> There's this piece of code in InstrStartNode:

> if (instr->need_timer && INSTR_TIME_IS_ZERO(instr->starttime))
> INSTR_TIME_SET_CURRENT(instr->starttime);
> else
> elog(DEBUG2, "InstrStartNode called twice in a row");

> but it should actually be like this

> if (instr->need_timer)
> {
> if (INSTR_TIME_IS_ZERO(instr->starttime))
> INSTR_TIME_SET_CURRENT(instr->starttime);
> else
> elog(DEBUG2, "InstrStartNode called twice in a row");
> }

Hm. It's a bit annoying that we can't detect the "called twice"
condition when !need_timer, but I suppose that any such bug would be a
caller logic error that would probably not be sensitive to need_timer,
so it's likely not worth adding overhead to handle that.

A bigger question is why this is elog(DEBUG2) and not elog(ERROR).
Had it been the latter, we'd have noticed the mistake immediately.
The current choice might be masking any caller-logic errors that
exist, too.

regards, tom lane