Re: Transaction commits VS Transaction commits (with parallel) VSquery mean time - Mailing list pgsql-hackers

From Haribabu Kommi
Subject Re: Transaction commits VS Transaction commits (with parallel) VSquery mean time
Date
Msg-id CAJrrPGcS8ugmd2ZYiaJfd7MuPgcnZQjpLOUV420bHLDRj8OSkQ@mail.gmail.com
Whole thread Raw
In response to Re: Transaction commits VS Transaction commits (with parallel) VSquery mean time  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: Transaction commits VS Transaction commits (with parallel) VSquery mean time
List pgsql-hackers

On Wed, Mar 20, 2019 at 7:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
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.
>

Thanks for the review.
 
@@ -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 understand your comment. current patch just avoids every transaction
that is used for the parallel operation.

With the use of 'is_parallel_worker' flag, it avoids only the actual transaction
that has done parallel operation (All supportive transactions are counted).
 
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?

As I said in my first mail, including pgAdmin4 also displays the TPS as stats on
the dashboard. Those TPS values are received from xact_commits column of the
pg_stat_database view.

With Inclusion of parallel worker transactions, the TPS will be a higher number,
thus user may find it as better throughput. This is the case with one of our tool.
The tool changes the configuration randomly to find out the best configuration
for the server based on a set of workload, during our test, with some configurations,
the TPS is so good, but the overall performance is slow as the system is having
less resources to keep up with that configuration.

Opinions?

Regards,
Haribabu Kommi
Fujitsu Australia

pgsql-hackers by date:

Previous
From: Chris Travers
Date:
Subject: Re: PostgreSQL pollutes the file system
Next
From: Haribabu Kommi
Date:
Subject: Re: Libpq support to connect to standby server as priority