Re: Patch to send transaction commit/rollback stats to the stats collector unconditionally.

Gurjeet Singh <gurjeet@singh.im> wrote:
> On Wed, Jul 2, 2014 at 3:49 PM, Kevin Grittner <kgrittn@ymail.com> wrote:
>> Gurjeet Singh <gurjeet@singh.im> wrote:
>>> Kevin Grittner <kgrittn@ymail.com> wrote:
>>>> I have reviewed this patch, and think we should do what the patch
>>>> is trying to do, but I don't think the submitted patch would
>>>> actually work.
>>>
>>> Just curious, why do you think it won't work.
>>
>> Because you didn't touch this part of the function:
>>
>>     /*
>>       * Send partial messages.  If force is true, make sure that any pending
>>       * xact commit/abort gets counted, even if no table stats to send.
>>       */
>>     if (regular_msg.m_nentries > 0 ||
>>         (force && (pgStatXactCommit > 0 || pgStatXactRollback > 0)))
>>         pgstat_send_tabstat(®ular_msg);
>>
>> The statistics would not actually be sent unless a table had been
>> accessed or it was forced because the connection was closing.
>
> I sure did! In fact that was the one and only line of code that was
> changed. It effectively bypassed the 'force' check if there were any
> transaction stats to report.

Sorry, I had too many versions of things sitting around and looked
at the wrong one.  It was the test at the top that you might not
get past to be able to report things below:

    /* Don't expend a clock check if nothing to do */
    if ((pgStatTabList == NULL || pgStatTabList->tsa_used == 0) &&
        !have_function_stats && !force)
        return;

The function stats might have gotten you past it for the particular
cases you were testing, but the problem could be caused without
that.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company