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

From: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Transaction commits VS Transaction commits (with parallel) VS query mean time
Date: 2019-03-21 06:17:33
Message-ID: CAJrrPGcS8ugmd2ZYiaJfd7MuPgcnZQjpLOUV420bHLDRj8OSkQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 20, 2019 at 7:38 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> On Sun, Feb 10, 2019 at 10:54 AM Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com> wrote:
> > On Sat, Feb 9, 2019 at 4:07 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2019-03-21 06:25:49 Re: Libpq support to connect to standby server as priority
Previous Message Chris Travers 2019-03-21 06:07:21 Re: PostgreSQL pollutes the file system