Re: add timing information to pg_upgrade

On Fri, Jul 28, 2023 at 5:21 AM Nathan Bossart <nathandbossart(at)gmail(dot)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