RE: "actual time" in QUERY PLAN for parallel operation when loops is bigger than 1

[tanghy.f...@fujitsu.com]

On Wednesday, November 17, 2021 4:19 PM, Guillaume Lelarge 
guilla...@lelarge.info wrote:
>> ->  Parallel Seq Scan on c  (cost=0.00..8591.67 rows=416667 width=0) (actual 
>> time=0.030..140.036 rows=33 loops=3)
>In my previous example, actual row number is 33*3=1e6(which is correct), 
>so I think the actual time is 140.036*3ms.
>Do your think the loops(3) has no meaning for parallel scan node when 
>calculate actual time?
>
>As far as I understand it, you have to multiply the number of rows by the 
>number of loops, but this doesn't apply to duration at least for parallel 
>>queries.

Yes, I got your point.
I’m not familiar with PostgreSQL planner/executor, but if the code is correct, 
then maybe some modification should be done at [1] as below:

Before:
Multiply by the loops value to get the total time actually spent in the node.

After:
Multiply by the loops value to get the total time actually spent in the node 
(for node in the parallel portion of the plan, this is not needed).

[1] https://www.postgresql.org/docs/14/using-explain.html

What do you think?

Regards,
Tang

RE: "actual time" in QUERY PLAN for parallel operation when loops is bigger than 1

[tanghy.f...@fujitsu.com]

Wednesday, November 17, 2021 6:29 AM, nikolai.berkoff  
wrote:
>
> Parallel query is explained in
> https://www.postgresql.org/docs/14/how-parallel-query-works.html and
> https://www.postgresql.org/docs/14/parallel-plans.html
> 
> The docs seem clear to me that as the nodes are executed in parallel then the
> time execution time is not 140.036*3. The 140.036 value is actual time the 
> Parallel
> Seq Scan nodes ran for but there were up to 2 running in parallel.

Thanks for your reply.
I read your references but still confused about the 'loops' in parallel query 
result.

> ->  Parallel Seq Scan on c  (cost=0.00..8591.67 rows=416667 width=0) (actual 
> time=0.030..140.036 rows=33 loops=3)
In my previous example, actual row number is 33*3=1e6(which is correct), so 
I think the actual time is 140.036*3ms.
Do your think the loops(3) has no meaning for parallel scan node when calculate 
actual time?

Regards,
Tang

> -Original Message-
> From: nikolai.berkoff 
> Sent: Wednesday, November 17, 2021 6:29 AM
> To: Tang, Haiying/唐 海英 ;
> pgsql-docs@lists.postgresql.org
> Subject: Re: "actual time" in QUERY PLAN for parallel operation when loops is
> bigger than 1
> 
> > https://www.postgresql.org/docs/14/using-explain.html
> > The doc says
> >
> 
> > "the loops value reports the total number of executions of the node, and the
> >
> 
> > actual time and rows values shown are averages per-execution. This is done
> >
> 
> > to make the numbers comparable with the way that the cost estimates are
> >
> 
> > shown."
> >
> 
> > But I found for parallel operation, the above description maybe not
> >
> 
> > correct.
> >
> 
> > For example
> >
> 
> > postgres=# create table c(id int);
> >
> 
> > CREATE TABLE
> >
> 
> > postgres=# insert into c select generate_series(1,100);
> >
> 
> > INSERT 0 100
> >
> 
> > postgres=# explain analyze select count(*) from c;
> >
> 
> > QUERY PLAN
> >
> ---
> ---
> ---
> ---
> ---
> ---
> ---
> ---
> ---
> ---
> >
> 
> > Finalize Aggregate (cost=10633.55..10633.56 rows=1 width=8) (actual
> >
> 
> > time=290.460..290.508 rows=1 loops=1)
> >
> 
> > -> Gather (cost=10633.33..10633.54 rows=2 width=8) (actual
> >
> 
> > time=289.605..290.484 rows=3 loops=1)
> >
> 
> > Workers Planned: 2
> >
> 
> > Workers Launched: 2
> >
> 
> > -> Partial Aggregate (cost=9633.33..9633.34 rows=1 width=8)
> >
> 
> > (actual time=188.336..188.337 rows=1 loops=3)
> >
> 
> > -> Parallel Seq Scan on c (cost=0.00..8591.67 rows=416667
> >
> 
> > width=0) (actual time=0.030..140.036 rows=33 loops=3)
> >
> 
> > Planning Time: 0.331 ms
> >
> 
> > Execution Time: 290.607 ms
> >
> 
> > (8 rows)
> >
> 
> > postgres=#
> >
> 
> > According to PG-doc, the "Parallel Seq Scan" node cost 140.036*3=420ms, but
> >
> 
> > the total cost for this SQL is only 290ms.
> >
> 
> > Is the output of this explain correct?
> 
> Parallel query is explained in
> https://www.postgresql.org/docs/14/how-parallel-query-works.html and
> https://www.postgresql.org/docs/14/parallel-plans.html
> 
> The docs seem clear to me that as the nodes are executed in parallel then the
> time execution time is not 140.036*3. The 140.036 value is actual time the 
> Parallel
> Seq Scan nodes ran for but there were up to 2 running in parallel.

[Doc] Tiny fix for regression tests example

[tanghy.f...@fujitsu.com]

Hi



When reading PG DOC, found some example code not correct as it said.

https://www.postgresql.org/docs/devel/regress-run.html



Here's a tiny fix in regress.sgml.



-make check PGOPTIONS="-c log_checkpoints=on -c work_mem=50MB"

+make check PGOPTIONS="-c geqo=off -c work_mem=50MB"



log_checkpoints couldn't be set in PGOPTIONS.

Replace log_checkpoints with geqo in the example code.



-make check EXTRA_REGRESS_OPTS="--temp-config=test_postgresql.conf"

+make check EXTRA_REGRESS_OPTS="--temp-config=$(pwd)/test_postgresql.conf"



User needs to specify $(pwd) to let the command execute as expected.



The above example code is added by Peter in PG14. So I think we need to apply 
this fix at PG14/master.

I proposed this fix at 
pgsql-docs@lists.postgresql.org at [1]. 
But no reply except Craig. So I please allow me to post the patch at Hackers’ 
mail list again in case the fix is missed.



[1] 
https://www.postgresql.org/message-id/OS0PR01MB6113FA937648B8F7A372359BFB009%40OS0PR01MB6113.jpnprd01.prod.outlook.com



Regards,

Tang



0001-minor-fix-for-regress-example.patch
Description: 0001-minor-fix-for-regress-example.patch

RE: doc: Document how to run regression tests with custom server settings

[tanghy.f...@fujitsu.com]

On Wednesday, June 30, 2021 11:15 AM, Craig Ringer 
craig.rin...@2ndquadrant.com wrote

>Yes, that'd make a lot of sense.
>
>PGOPTIONS is more suited for client settings.
>
>Willing to cook up a quick patch?

Thanks for replying. Here is my patch to fix the doc issue. Kindly to take at 
check.

Regards,
Tang


0001-minor-fix-for-regress-example.patch
Description: 0001-minor-fix-for-regress-example.patch

RE: doc: Document how to run regression tests with custom server settings

[tanghy.f...@fujitsu.com]

Hi

I was doing regression testing according to the PG-doc at [1]. 
The modification at 854434c5 seems not correct, could you please take a check 
at it? 

>make check PGOPTIONS="-c log_checkpoints=on -c work_mem=50MB"

The above command reported an error like this:
failed: FATAL:  parameter "log_checkpoints" cannot be changed now

IMHO, as a sighup GUC, log_checkpoints can't be set in PGOPTIONS, is that 
correct?

>make check EXTRA_REGRESS_OPTS="--temp-config=test_postgresql.conf"

pg_regress: could not open "test_postgresql.conf" to read extra config: No such 
file or directory

I fixed above problem using command like this:
make check EXTRA_REGRESS_OPTS="--temp-config=$(pwd)/test_postgresql.conf"

Maybe we can change doc description to tell user he/she should specify the 
location of " test_postgresql.conf", thoughts?

[1] https://www.postgresql.org/docs/devel/regress-run.html

Regards,
Tang

RE: pg_type_d.h location incorrect

[tanghy.f...@fujitsu.com]

On Thursday, May 27, 2021 11:27 PM, Tom Lane  wrote
>No ... "patch -p1 <~/improve-pgservice-docs.patch" works fine for me.

Oh, never tried "patch -p1" command before but it worked. Thanks!!
BTW, here is my previous operation to apply your patch(failed, which is strange 
IMO)

$ git apply --3way improve-pgservice-docs.patch
error: patch failed: doc/src/sgml/libpq.sgml:8091
Falling back to three-way merge...
error: patch failed: doc/src/sgml/libpq.sgml:8091
error: doc/src/sgml/libpq.sgml: patch does not apply

>grepping finds lots of occurrences of '~/' and
>only one of '$HOME/'.  Seems like we ought to change that one:
>
>$ grep -r 'HOME/' .
>./config.sgml:This writes out files to 
>$HOME/.debug/jit/; the

Agreed, thanks for the comment.
Attached a patch to combine the above fix in your patch.
Also fixed a typo in install-windows.sgml.
Also fixed pgbuffercache.sgml as Michael commented at [1].
 
[1] https://www.postgresql.org/message-id/YK4A4g07AvGa9FVF%40paquier.xyz

Regards,
Tang


improve-pgservice-docs-etc.patch
Description: improve-pgservice-docs-etc.patch

RE: pg_type_d.h location incorrect

[tanghy.f...@fujitsu.com]

On Thursday, May 27, 2021 10:58 AM, Tom Lane  wrote
>So I ended up with the attached --- what do you think?

The doc-fix LGTM. 
But I couldn't apply the patch at HEAD(2941138e60). Maybe you did the fix at a 
branch other than HEAD?

>Also, looking at this, I note that "~/.pg_service.conf" is still a
>platform-ism.  We could make that slightly better by writing
>"$HOME/.pg_service.conf", but is that good enough?

Agreed. Besides, A lot of places in current Doc have been useing ~/.pg_*** 
already.
IMHO, it's good to keep the consistency.

Regards,
Tang

RE: pg_type_d.h location incorrect

[tanghy.f...@fujitsu.com]

On Wednesday, May 26, 2021 12:20 PM, Tom Lane  wrote
>In the first place, it's somewhere between unhelpful and flat out wrong for
>people on Windows, where the backtick notation doesn't work (AFAIK).
>In the second, it will distract almost every user, who will need
>to stop for a second or two to think about what context they would
>use backticks in, and about what pg_config is, and whether the right
>pg_config is even in their $PATH, and about why --pkgincludedir is
>the right switch.  If they don't already have a bunch of those facts
>swapped in, it will take a lot longer than a second or two to figure
>out what is meant here.  That seems completely out of proportion to
>the value of having this passing mention be pedantically correct.

Thanks for your comments. 
You're right, backtick notation doesn't work at my windows machine.
But I made the fix in accordance with the pg-doc as below. So maybe we need a 
fix there, too?

https://www.postgresql.org/docs/current/libpq-pgservice.html
a system-wide file at `pg_config --sysconfdir`/pg_service.conf

Regards,
Tang

RE: pg_type_d.h location incorrect

[tanghy.f...@fujitsu.com]

Hi

Attached a patch to fix the incorrect location description of pg_type_d.h 
posted by myself at [1].

[1] 
https://www.postgresql.org/message-id/162149020918.26174.7150424047314144...@wrigleys.postgresql.org

Regards,
Tang


0001-dir-fix-for-pg_type_d.h.patch
Description: 0001-dir-fix-for-pg_type_d.h.patch

RE: typo in doc for "Miscellaneous Coding Conventions"

[tanghy.f...@fujitsu.com]

>"were not" is typical usage when stating a contrary-to-fact hypothetical.
> If the implementation (or you) didn’t do X, then Y bad thing could happen.

Really thanks for your kindly reply and explain.
Please forgive me for my poor English. Anyway, got it.

Regards,
Tang

doc-fix-for-POSIX-Time-Zone-Specifications

[tanghy.f...@fujitsu.com]

Hi

Attached a patch to delete "CURRENT(as of 2020) " description in POSIX Time 
Zone Specifications.
I'm not a native English speaker, if my fix is not appropriate or insufficient, 
please be kind to share your thought with me.

Regards,
Tang


0001-doc-fix-for-POSIX-Time-Zone-Specifications.patch
Description: 0001-doc-fix-for-POSIX-Time-Zone-Specifications.patch

RE: [DOC] pg_stat_replication_slots representation style inconsisitant

[tanghy.f...@fujitsu.com]

On Tuesday, May 11, 2021 2:42 PM, Michael Paquier  wrote:
>I was looking at this thread, and please note that there is no need to
>do anything here as the WAL prefetch has been reverted for now as of
>c2dc1934.

Thanks for the updating. Got it.

Regards,
Tang

RE: Unsupported version mentioned at Using EXPLAIN

[tanghy.f...@fujitsu.com]

Tom Lane  wrote:
> On 2021-May-07, Alvaro Herrera  wrote:
>> I think there's more to this than just changing or removing the version
>> number; a better approach might be to update the documentation so that
>> it matches what current versions do.

>Yeah.  The reason why a specific version is mentioned is exactly that
>the details tend to change.

Thanks for your kindly explanation on the usage of version number in Doc.
In fact, I noticed the different context in the examples among different PG 
version like "increment sort" which was newly introduced in PG13. 
The fix I mentioned at the previous mail(change "using 9.3 development sources" 
to " using current development sources") is meant to tell users to refer to the 
"current" PG source. E.G. when a user looks at PG13.2's doc, "current" = PG13.2.
Since I do think your advice on the fix is more reasonable and friendly, I 
followed your hint to modify the Doc in the attached patch.
Please tell me if there's anything else insufficient.

Regards,
Tang


0001-Using-current-version-in-Using-EXPLAIN-Doc.patch
Description: 0001-Using-current-version-in-Using-EXPLAIN-Doc.patch

Unsupported version mentioned at Using EXPLAIN

[tanghy.f...@fujitsu.com]

Hi

When referring Doc, found one place mentioned "using 9.3 development sources." 
at [1]. Which I think 9.3 means PG9.3

[1]
https://www.postgresql.org/docs/current/using-explain.html

It seems to show up since PG9.2 and changed to 9.3 at PG9.3. However, since 9.3 
nobody updates the version anymore. 
Maybe we can just remove the specific PG version. Say "using current 
development sources." Thoughts?

Regards,
Tang

RE: [DOC] pg_stat_prefetch_recovery representation style inconsisitant

[tanghy.f...@fujitsu.com]

Sorry, wrong title name. I fixed the view representation of 
"pg_stat_prefetch_recovery". Fix it.

Regards,
Tang

RE: [DOC] pg_stat_replication_slots representation style inconsisitant

[tanghy.f...@fujitsu.com]

On Friday, April 30, 2021 12:38 PM, Hou, Zhijie/侯 志杰  
wrote
>I noticed one more thing.
>It seems the column " stats_reset | timestamp with time zone " is not 
>listed in the pg_stat_prefetch_recovery view.
>Should we add this column too ?

Indeed, column added. Thanks. 

Regards,
Tang


0001-pg_stat_prefetch_recovery_doc_v2.patch
Description: 0001-pg_stat_prefetch_recovery_doc_v2.patch

[DOC] pg_stat_replication_slots representation style inconsisitant

[tanghy.f...@fujitsu.com]

Hi 

When reading the manual doc, I found an inconsistent representation style for 
the newly added pg_stat_prefetch_recovery view in PG14 at [1].
Same problem seems to be reported by Noriyoshi Shinoda who also proposed a 
patch to fix the problem at[2].
I tried to fix the problem in a way different than Noriyoshi Shinoda which I 
think is more consistent with the existing specific view introduction in the 
same page.
Please take the attached patch as your reference.

[1]
https://www.postgresql.org/docs/devel/monitoring-stats.html#MONITORING-PG-STAT-SUBSCRIPTION

[2]
https://www.postgresql.org/message-id/TU4PR8401MB1152945E4FD45E99C3F52B94EE4F9%40TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM

Regards,
Tang


0001-pg_stat_prefetch_recovery_doc.patch
Description: 0001-pg_stat_prefetch_recovery_doc.patch