Thread: Documentation of underlying functions for statistics views
Currently, some of the per-column entries in monitoring.sgml mention the
underlying stats function; but the coverage is incomplete, and it seems
very cluttering anyway. I propose that we remove all that and instead
just suggest that people examine the definition of the view if they want
to know the names of the underlying functions.
regards, tom lane
On Sat, Apr 28, 2012 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Currently, some of the per-column entries in monitoring.sgml mention the > underlying stats function; but the coverage is incomplete, and it seems > very cluttering anyway. I propose that we remove all that and instead > just suggest that people examine the definition of the view if they want > to know the names of the underlying functions. I agree that most of the plain descriptions like: | This value can also be returned by directly calling the | pg_stat_get_db_blocks_hit function. are pretty worthless, because you can trivially figure that out the function name from the view definition, and there's not any other useful information in that snippet. And you'd have to do some futzing to figure out the right inputs to the function if you wanted to call it directly, anyway. I did find it marginally useful to see clarifications of milliseconds vs. microseconds (e.g. "block_write_time" column). Yeah, you could figure out that based on the " / 1000" in the view description, but it'd be a bit less obvious. So maybe keep just those? Josh
On Sat, Apr 28, 2012 at 23:24, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Currently, some of the per-column entries in monitoring.sgml mention the > underlying stats function; but the coverage is incomplete, and it seems > very cluttering anyway. I propose that we remove all that and instead > just suggest that people examine the definition of the view if they want > to know the names of the underlying functions. I could've sworn I suggested this before and got shut down. However ,since I can't find a reference to that in my sent email, I'm just going to +1 this suggestion instead :-) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Josh Kupershmidt <schmiddy@gmail.com> writes:
> On Sat, Apr 28, 2012 at 2:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Currently, some of the per-column entries in monitoring.sgml mention the
>> underlying stats function; but the coverage is incomplete, and it seems
>> very cluttering anyway. I propose that we remove all that and instead
>> just suggest that people examine the definition of the view if they want
>> to know the names of the underlying functions.
> I did find it marginally useful to see clarifications of milliseconds
> vs. microseconds (e.g. "block_write_time" column). Yeah, you could
> figure out that based on the " / 1000" in the view description, but
> it'd be a bit less obvious. So maybe keep just those?
There's an existing paragraph that explains that the underlying
functions take backend ID or table/index/function OID. I was thinking
of expanding that to add the recommendation to look at the view
definitions. We could also add a sentence there that says that for
timing columns, the underlying function reports integer microseconds
which are converted to milliseconds by the view. I think that's better
than repeating the info for each such column.
Alternatively, we could bite the bullet and change all those functions
to report float8 msec now, instead of putting that change off ;-).
I'm actually not sure what the point of waiting on such a change is;
it's not going to become any less painful, especially not if we are
encouraging people to develop custom views that use the functions
directly. But this is the wrong list to debate such a proposal.
regards, tom lane
On Sun, Apr 29, 2012 at 8:03 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Josh Kupershmidt <schmiddy@gmail.com> writes: >> I did find it marginally useful to see clarifications of milliseconds >> vs. microseconds (e.g. "block_write_time" column). Yeah, you could >> figure out that based on the " / 1000" in the view description, but >> it'd be a bit less obvious. So maybe keep just those? > > There's an existing paragraph that explains that the underlying > functions take backend ID or table/index/function OID. I was thinking > of expanding that to add the recommendation to look at the view > definitions. We could also add a sentence there that says that for > timing columns, the underlying function reports integer microseconds > which are converted to milliseconds by the view. I think that's better > than repeating the info for each such column. Yeah, that sounds fine. The paragraph I think you're talking about claims "These functions are listed in Table 27-13", but Table 27-13 only has a few functions (e.g. pg_stat_get_db_block_time_read() is missing). Maybe that paragraph could clarify that it's talking generally about all the views/functions on that page. Josh
Josh Kupershmidt <schmiddy@gmail.com> writes:
> Yeah, that sounds fine. The paragraph I think you're talking about
> claims "These functions are listed in Table 27-13", but Table 27-13
> only has a few functions (e.g. pg_stat_get_db_block_time_read() is
> missing). Maybe that paragraph could clarify that it's talking
> generally about all the views/functions on that page.
It seemed to me that the only view-underlying function that needs
explicit documentation is pg_stat_get_activity(), because the fact
that you can pass it a PID isn't obvious from the view's usage.
So that's the only one I left in table 27-13. However, I realized
that the stats functions that we used in pg_stat_activity before 8.4
are not documented at all with this approach, so I made another table
27-14 to cover them. I'm not sure if we should mark them deprecated
or not; thoughts?
Anyway, this is committed and up on the devel docs website, if
you want to take a look.
regards, tom lane
On Sun, Apr 29, 2012 at 2:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Josh Kupershmidt <schmiddy@gmail.com> writes:
>> Yeah, that sounds fine. The paragraph I think you're talking about
>> claims "These functions are listed in Table 27-13", but Table 27-13
>> only has a few functions (e.g. pg_stat_get_db_block_time_read() is
>> missing). Maybe that paragraph could clarify that it's talking
>> generally about all the views/functions on that page.
>
> It seemed to me that the only view-underlying function that needs
> explicit documentation is pg_stat_get_activity(), because the fact
> that you can pass it a PID isn't obvious from the view's usage.
> So that's the only one I left in table 27-13. However, I realized
> that the stats functions that we used in pg_stat_activity before 8.4
> are not documented at all with this approach, so I made another table
> 27-14 to cover them. I'm not sure if we should mark them deprecated
> or not; thoughts?
>
> Anyway, this is committed and up on the devel docs website, if
> you want to take a look.
I think you left an extra
<entry>
</entry>
in the description for pg_stat_get_backend_start(integer), around line
1712 of monitoring.sgml. Other than that, I liked the changes.
Josh
Josh Kupershmidt <schmiddy@gmail.com> writes:
> I think you left an extra
> <entry>
> </entry>
> in the description for pg_stat_get_backend_start(integer), around line
> 1712 of monitoring.sgml. Other than that, I liked the changes.
Wups, you're right. Will fix in my next set of changes. Thanks
for catching that.
regards, tom lane