Thread: 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
Attachment
Jeff Janes <jeff.janes@gmail.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@sss.pgh.pa.us> wrote:
Jeff Janes <jeff.janes@gmail.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