On Fri, Jul 28, 2023 at 5:21 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> This information can be used to better understand where the time is going
> and to validate future improvements. I'm open to suggestions on formatting
> the timing information, assuming folks are interested in this idea.
>
> Thoughts?
+1 for adding time taken.
Some comments on the patch:
1.
+ gettimeofday(&step_start, NULL);
+ gettimeofday(&step_end, NULL);
+ start_ms = (step_start.tv_sec * 1000L) + (step_start.tv_usec / 1000L);
+ end_ms = (step_end.tv_sec * 1000L) + (step_end.tv_usec / 1000L);
+ elapsed_ms = end_ms - start_ms;
How about using INSTR_TIME_SET_CURRENT, INSTR_TIME_SUBTRACT and
INSTR_TIME_GET_MILLISEC macros instead of gettimeofday and explicit
calculations?
2.
> Checking database user is the install user ok (took 3 ms)
> Checking database connection settings ok (took 4 ms)
+ report_status(PG_REPORT, "ok (took %ld ms)", elapsed_ms);
I think it's okay to just leave it with "ok \t %ld ms", elapsed_ms);
without "took". FWIW, pg_regress does that way, see below:
ok 2 + boolean 50 ms
ok 3 + char 32 ms
ok 4 + name 33 ms
> Performing Consistency Checks
> -----------------------------
> Checking cluster versions ok (took 0 ms)
> Checking database user is the install user ok (took 3 ms)
> Checking database connection settings ok (took 4 ms)
> Checking for prepared transactions ok (took 2 ms)
> Checking for system-defined composite types in user tables ok (took 82 ms)
> Checking for reg* data types in user tables ok (took 55 ms)
Just curious, out of all the reported pg_upgrade prep_status()-es,
which ones are taking more time?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com