Re: Access statistics

Tom Lane wrote:
> Jan Wieck <JanWieck@Yahoo.com> writes:
> > Here it comes.
>
> Hm.  I really don't like the way you've uglified the heap_fetch
> interface; can't that be done better?  It's just plain bizarre that
> heap_fetch is now responsible for counting stats for index scans.
> And surely this bit of code is wrong:
>
>         *userbuf = buffer;
> +
> +       if (iscan != NULL)
> +            pgstat_count_heap_fetch(&iscan->xs_pgstat_info);
> +       else
> +            pgstat_count_heap_fetch(&relation->pgstat_info);
>    }
>
> You can't count heapscan info via the relcache entry --- what if there's
> more than one heapscan in progress on the same rel?  What if the
> relcache entry gets rebuilt by SI invalidation?  I don't think the stats
> stuff should touch the relcache at all.

    The  counting end's up in one and the same global slot of the
    tabstat messages. These  slots  are  collected  per  frontend
    command  and are identified by the tables oid. Does it matter
    how often the pointer to that slot is  updated  to  the  same
    value?

>
> There's been talk in the past of trying to unify the heapscan and
> indexscan APIs, so that we wouldn't need ugliness like the two sets of
> coding in AttrDefaultFetch and similar places.  Maybe it's time to bite
> that bullet, if it'd provide a cleaner place to put the stats calls.

    Exactly  that's  the  problem. Only heap_fetch() knows if the
    tid passed in is visible or not, so right  now  the  counting
    could  be  done there or in all the places where heap_fetch()
    is called.

>
> Also, I do not like the pgStatPmPipe mechanism for detecting postmaster
> shutdown.  That eats two file descriptors in every backend, which is a
> pretty substantial waste of resources.  Why not just have the postmaster
> send a signal to the stats collector when it's time to shut down?

    Don't kill -9 the postmaster :-)

    Well, we could do the signal *plus* having the child checking
    from  time to time with a zero kill if the postmaster died to
    suizide in case.

>
> BTW, PG_FUNCTION_INFO_V1() is not needed for built-in functions.

    Survived somehow from  the  time  where  they  resided  in  a
    loadable module.


Jan


--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #



_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com