On Tue, Dec 15, 2015 at 5:53 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
> I wrote:
>> Why would the likelyhood of an issue be small here?
>
> The time to update one stat (<< 100 cycles ?) to the time to do a
> transaction with the database (typically Y ms), so the likelyhood of two
> thread to update the very same stat at the same time is probably under
> 1/10,000,000. Even if it occurs, then one stat is slightly false, no big
> deal. So I think the potential slowdown induced by a mutex is not worth it,
> so I a comment instead.
OK, got it and agreed.
>> + /* print NaN if no transactions where executed */
>> + double latency = ss->sum / ss->count;
>> This does not look like a good idea, ss->count can be 0.
>
>
> "sum" is a double so count is converted to 0.0, 0.0/0.0 == NaN, hence the
> comment.
PG code usually avoids that, and I recall static analyze tools type
coverity complaining that this may lead to undefined behavior. While I
agree that this would lead to NaN...
>> It seems also that it would be a good idea to split the patch into two
>> parts:
>> 1) Refactor the code so as the existing test scripts are put under the
>> same umbrella with addScript, adding at the same time the new option
>> -b.
>> 2) Add the weight facility and its related statistics.
>
>
> Sigh. The patch & documentation are probably not independent, so that would
> make two dependent patches, probably.
I am not really saying so, it seems just that doing the refactoring
(with its related docs), and then add the extension for the weight
(with its docs) is more natural than doing both things at the same
time.
--
Michael