Re: Re[2]: [PATCH] pg_stat_statements: add last_execution_start column - Mailing list pgsql-hackers

From Pavlo Golub
Subject Re: Re[2]: [PATCH] pg_stat_statements: add last_execution_start column
Date
Msg-id CAK7ymcJpxfgu3q2XXwuuifrtkV6SqU9YbAt1=Ut_DdJ8wjS99A@mail.gmail.com
Whole thread Raw
In response to Re: Re[2]: [PATCH] pg_stat_statements: add last_execution_start column  (Sami Imseih <samimseih@gmail.com>)
List pgsql-hackers
>
> I think adding to queryDesc->EState just like we do for other fields
> that are consumed by pg_stat_statements, i.e. es_processed,
> es_parallel_workers_to_launch, etc. is the proper pattern.

Yeah, makes sense. Thanks.

> If we track the start time in queryDesc->EState, this will not be a
> problem. For utility
> statements, we can rely on a direct call to GetCurrentStatementStartTimestamp()

Thank you for the review.  v3 addresses all concerns, I hope.

I switched from a bounded static array to statement start timestamp
stored in EState.
The field is set in pgss_ExecutorStart immediately after
standard_ExecutorStart creates the estate.

All pg_stat_statements regression tests pass (16/16).

Benchmark (16-vCPU, pgbench -c8 -j4 -T60):
  master HEAD:  ~4691 TPS (runs: 4849, 4588, 4635)
  Patched v3: ~4744 TPS (runs: 4870, 4735, 4627)
Difference: ~1.1%

No measurable overhead.

The latest v3 patch attached.

>
> We also need to document that for toplevel = false statements, this
> execution_start time
> is that of the top level statement. I think that is acceptable.

Docs updated

>
> --
> Sami Imseih
> Amazon Web Services (AWS)

Attachment

pgsql-hackers by date:

Previous
From: Kirill Reshke
Date:
Subject: Re: GetCachedPlan() refactor: move execution lock acquisition out
Next
From: Melanie Plageman
Date:
Subject: Re: Two issues leading to discrepancies in FSM data on the standby server