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

On 18/07/18 22:56, Fabien COELHO wrote:
> 
> Hello Heikki,
> 
>>> Yep. The attached version does only the tailing stuff under -R and not all
>>> threads were stopped on errors, with comments to tell about the why.
>>
>> 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:

> 		for (i = 0; i < nstate; i++)
> 		{
> 			CState	   *st = &state[i];
> 
> 			if (st->state == CSTATE_THROTTLE && timer_exceeded)
> 			{
> 				/* interrupt client that has not started a transaction */
> 				st->state = CSTATE_FINISHED;
> 				finishCon(st);
> 				remains--;
> 			}
> 			else if (st->state == CSTATE_SLEEP || st->state == CSTATE_THROTTLE)
 >			...

As soon as the -T timer is exceeded, the above code will close all 
connections that are in CSTATE_THROTTLE state.

- Heikki