Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Re: [COMMITTERS] pgsql: Remove pgbench "progress" test pending solution of its timing is (fwd)
Date: 2018-07-18 21:03:12
Message-ID: 70277469-1f81-0216-6ac7-c6081c141b10@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 18/07/18 23:29, Fabien COELHO wrote:
>>>> Hmm. How about we just remove this special case from doCustom():
>>>>
>>>>> case CSTATE_START_THROTTLE:
>>>>> // ...
>>>>> if (duration > 0 && st->txn_scheduled > end_time)
>>>>> {
>>>>> st->state = CSTATE_FINISHED;
>>>>> break;
>>>>> }
>>>>
>>>> That way, we let the client go into CSTATE_THROTTLE state, even though
>>>> we know that the timer will run out before we reach txn_scheduled.
>>>> Then it will work the way we want, right? One small difference is that
>>>> then the clients will keep the connections open longer, until the
>>>> timer expires, but I think that's reasonable. Less surprising than the
>>>> current behavior, even.
>>>
>>> Hmmm... in this instance, and if this test is removed, ISTM that it can
>>> start the transaction, re-establishing a connection under --connect, and
>>> the transaction will run to its end even if it is beyond the expected end
>>> of run. So removing this test does not seem desirable.
>>
>> Can you elaborate? I don't think that's how it works. In threadRun(), we have
>> this:
>
> The state path I want to avoid is, without getting out of doCustom, is:
>
> CHOOSE_SCRIPT:
> -> START_THROTTLE (i.e. under -R)
> START_THROTTLE:
> update txn_schedule, which happen to be after end_time,
> i.e. the transaction is scheduled after the expected end of the run.
> but if the condition is removed, then proceed directly to
> -> THROTTLE
> THROTTLE:
> if now has passed txn_schedule (we are late), no return, proceed
> directly to -> START_TX
> START_TX:
> -> START_COMMAND
> START_COMMAND: executed... & "return" on SQL, but it is too late
> for stopping the tx now, it has started.
>
> Maybe I missed something, but it looks to me that we can get in the
> situation where a transaction scheduled beyond the end of run is started
> anyway if it happens that it was a little late.
>
>> [... threadRun ...]
>> As soon as the -T timer is exceeded, the above code will close all
>> connections that are in CSTATE_THROTTLE state.
>
> So threadRun() would not have the opportunity to stop the scheduled
> transaction, even if beyond the end of run, because it would not have got
> out of doCustom, in the case I outlined above.

I see. Instead of moving to FINISHED state, then, we could stay in
THROTTLE state, and 'return' out of doCustom(), to give the code in
threadRun() a chance to detect that the timer is up. Something like the
attached. (I moved the check after the check for latency_limit, because
that code updates txn_scheduled. Seems more like a more correct place,
although that's as a separate issue.)

- Heikki

Attachment Content-Type Size
dont-finish-pgbench-too-early-1.patch text/x-patch 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-07-18 21:05:44 Re: psql's \d versus included-index-column feature
Previous Message Tom Lane 2018-07-18 20:46:16 Re: psql's \d versus included-index-column feature