Re: Transaction commits VS Transaction commits (with parallel) VSquery mean time

On Sun, Feb 10, 2019 at 10:54 AM Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> On Sat, Feb 9, 2019 at 4:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> I don't think so.  It seems to me that we should consider it as a
>> single transaction.  Do you want to do the leg work for this and try
>> to come up with a patch?  On a quick look, I think we might want to
>> change AtEOXact_PgStat so that the commits for parallel workers are
>> not considered.  I think the caller has that information.
>
>
> I try to fix it by adding a check for parallel worker or not and based on it
> count them into stats. Patch attached.
>

@@ -2057,14 +2058,18 @@ AtEOXact_PgStat(bool isCommit)
 {
..
+ /* Don't count parallel worker transactions into stats */
+ if (!IsParallelWorker())
+ {
+ /*
+ * Count transaction commit or abort.  (We use counters, not just bools,
+ * in case the reporting message isn't sent right away.)
+ */
+ if (isCommit)
+ pgStatXactCommit++;
+ else
+ pgStatXactRollback++;
+ }
..
}

I wonder why you haven't used the 'is_parallel_worker' flag from the
caller,  see CommitTransaction/AbortTransaction?  The difference is
that if we use that then it will just avoid counting transaction for
the parallel work (StartParallelWorkerTransaction), otherwise, it
might miss the count of any other transaction we started in the
parallel worker for some intermediate work (for example, the
transaction we started to restore library and guc state).

I think it boils down to whether we want to avoid any transaction that
is started by a parallel worker or just the transaction which is
shared among leader and worker.  It seems to me that currently, we do
count all the internal transactions (started with
StartTransactionCommand and CommitTransactionCommand) in this counter,
so we should just try to avoid the transaction for which state is
shared between leader and workers.

Anyone else has any opinion on this matter?