On Fri, Jul 28, 2023 at 5:21 AM Nathan Bossart <nathandboss...@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
