Hi Bertrand-san,
Thank you for your review!
> === 1
>
> -pgstat_count_lock_waits(uint8 locktag_type, long msecs)
> +pgstat_count_lock_waits(uint8 locktag_type, double msecs)
>
> What about keeping the long and rename to usecs?
Agreed. Fixed.
> === 2
>
> - pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
> + pgstat_count_lock_waits(locallock->tag.lock.locktag_type,
> + (double) msecs + (double) usecs / 1000.0);
>
> would:
>
> - msecs = secs * 1000 + usecs / 1000;
> - usecs = usecs % 1000;
>
> /* Increment the lock statistics counters if done waiting. */
> if (myWaitStatus == PROC_WAIT_STATUS_OK)
> - pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
> + pgstat_count_lock_waits(locallock->tag.lock.locktag_type, secs * 1000000 + usecs);
> +
> + msecs = secs * 1000 + usecs / 1000;
> + usecs = usecs % 1000;
Agreed. Fixed.
> === 3
>
> - values[i++] = Int64GetDatum(lck_stats->wait_time);
> + values[i++] = Float8GetDatum(lck_stats->wait_time);
>
> Then, what about doing:
>
> values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
>
> instead?
Agreed. Fixed.
> === 4
>
> and instead of:
>
> - PgStat_Counter wait_time; /* time in milliseconds */
> + double wait_time; /* time in milliseconds */
>
> only change the comment here: to microseconds (but keep PgStat_Counter as type).
>
> The idea being to keep the PgStat_Counter type, the long parameter type and
> do the conversion at display time.
Agreed. Fixed.
v2 attached.
Regards,
Tatsuya Kawata
