Thanks for taking another look!
On Tue, Mar 7, 2023 at 10:52 AM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
> On 3/6/23 5:30 PM, Melanie Plageman wrote:
> > Thanks for the review!
> >
> > On Tue, Feb 28, 2023 at 4:49 AM Drouvot, Bertrand
> > <bertranddrouvot.pg@gmail.com> wrote:
> >> On 2/26/23 5:03 PM, Melanie Plageman wrote:
> >>> The timings will only be non-zero when track_io_timing is on
> >>
> >> That could lead to incorrect interpretation if one wants to divide the timing per operations, say:
> >>
> >> - track_io_timing is set to on while there is already operations
> >> - or set to off while it was on (and the number of operations keeps growing)
> >>
> >> Might be worth to warn/highlight in the "track_io_timing" doc?
> >
> > This is a good point. I've added a note to the docs for pg_stat_io.
>
> Thanks!
>
> Now I've a second thought: what do you think about resetting the related number
> of operations and *_time fields when enabling/disabling track_io_timing? (And mention it in the doc).
>
> That way it'd prevent bad interpretation (at least as far the time per operation metrics are concerned).
>
> Thinking that way as we'd loose some (most?) benefits of the new *_time columns
> if one can't "trust" their related operations and/or one is not sampling pg_stat_io frequently enough (to discard the
samples
> where the track_io_timing changes occur).
>
> But well, resetting the operations could also lead to bad interpretation about the operations...
>
> Not sure about which approach I like the most yet, what do you think?
Oh, this is an interesting idea. I think you are right about the
synchronization issues making the statistics untrustworthy and, thus,
unuseable.
Building on your idea, what if we had the times be NULL instead of zero
when track_io_timing is disabled? Then as you suggested, when you enable
track_io_timing, it resets the IOOp counts and starts the times off at
zero. However, disabling track_io_timing would only NULL out the times
and not zero out the counts.
We could also, as you say, log these events.
- Melanie