Hi,
On 3/21/23 2:16 PM, Imseih (AWS), Sami wrote:
This indeed feels a bit more natural seen from here, after looking at
the code paths using an Instrumentation in the executor and explain,
for example. At least, this stresses me much less than adding 16
bytes to EState for something restricted to the extended protocol when
it comes to monitoring capabilities.
Attached is the patch that uses Instrumentation.
Thanks, I think this new approach makes sense.
- const BufferUsage *bufusage,
+ int64 calls, const BufferUsage
*bufusage,
What about using an uint64 for calls? That seems more appropriate to me (even if
queryDesc->totaltime->calls will be passed (which is int64), but that's already
also the case for the "rows" argument and queryDesc->totaltime->rows_processed)
@@ -88,6 +88,8 @@ typedef struct Instrumentation
double nfiltered2; /* # of tuples removed by
"other" quals */
BufferUsage bufusage; /* total buffer usage */
WalUsage walusage; /* total WAL usage */
+ int64 calls; /* # of total calls to
ExecutorRun */
+ int64 rows_processed; /* # of total rows processed in
ExecutorRun */
I'm not sure it's worth mentioning that the new counters are "currently" used
with the ExecutorRun.
What about just "total calls" and "total rows processed" (or "total rows", see
below)?
Also, I wonder if "rows" (and not rows_processed) would not be a better naming.
Those last comments regarding the Instrumentation are done because ISTM that at
the end their usage
could vary depending of the use case of the Instrumentation.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
