trivial patch for dynahash logging

under HASH_STATISTICS, the output reporting "this HTAB" upon destruction is
pretty useless. Which HTAB would this one be? It is not necessarily the
most recently created one.

This makes it output the %p to the actual HTAB, so it can be matched up
with the logging of the creation.

I'm not sure why it bypasses elog. Is that so it can run during start-up
before elog is active? I'd like to make it go through elog so that
log_line_prefix are applied, but then it would no longer be a trivial patch.

Cheers,

Jeff

Jeff Janes <jeff(dot)janes(at)gmail(dot)com> writes:
> under HASH_STATISTICS, the output reporting "this HTAB" upon destruction is
> pretty useless. Which HTAB would this one be? It is not necessarily the
> most recently created one.

> This makes it output the %p to the actual HTAB, so it can be matched up
> with the logging of the creation.

Seems reasonable, although I wonder why neither of them print the tabname
string. More generally, I see no calls to hash_stats() anywhere except
in hash_destroy(), so wouldn't you need a rather larger patch to make it
actually do anything useful?

> I'm not sure why it bypasses elog. Is that so it can run during start-up
> before elog is active? I'd like to make it go through elog so that
> log_line_prefix are applied, but then it would no longer be a trivial patch.

A quick dig in our git history shows that it was like that in Postgres95,
so the answer is most likely "this code predates elog()". I would think
it'd be a safe assumption that elog is initialized before any hashtables
are created, but certainly you could test that pretty quickly ...

regards, tom lane

On Sat, Sep 27, 2014 at 3:09 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Jeff Janes <jeff(dot)janes(at)gmail(dot)com> writes:
> > under HASH_STATISTICS, the output reporting "this HTAB" upon destruction
> is
> > pretty useless. Which HTAB would this one be? It is not necessarily the
> > most recently created one.
>
> > This makes it output the %p to the actual HTAB, so it can be matched up
> > with the logging of the creation.
>
> Seems reasonable, although I wonder why neither of them print the tabname
> string. More generally, I see no calls to hash_stats() anywhere except
> in hash_destroy(), so wouldn't you need a rather larger patch to make it
> actually do anything useful?
>

I also wondered about the tabname string. Maybe it didn't exist back when
this was added.

I'm not sure where else it should be called other than from hash_destroy.
But if I had a need to call it from elsewhere for some investigative
purpose, I would add the call there and recompile. Once the base
functionality is there, it is easy to enough to add invocations.

But just having it report at destruction was useful enough to convince me
that hash collisions were not the problem, and I had to look elsewhere (not
in dynahash) for this particular problem. That is a pretty useful thing to
know. I probably could have made the same conclusion without the %p, but
would not have had much confidence in it.

(For the curious, my tentative conclusion was that dynahash is as about as
efficient as can be expected here, but the bitmap index scan cost estimate
is overly optimistic about how efficient that is.)

Cheers,

Jeff