Re: add timing information to pg_upgrade

On Fri, Jul 28, 2023 at 01:10:14PM +0530, Bharath Rupireddy wrote:
> How about using INSTR_TIME_SET_CURRENT, INSTR_TIME_SUBTRACT and
> INSTR_TIME_GET_MILLISEC macros instead of gettimeofday and explicit
> calculations?

That seems like a good idea.

> + 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:

I'm okay with simply adding the time. However, I wonder if we want to
switch to seconds, minutes, hours, etc. if the step takes longer. This
would add a bit of complexity, but it would improve human readability.
Alternatively, we could keep the code simple and machine readable by always
using milliseconds.

> Just curious, out of all the reported pg_upgrade prep_status()-es,
> which ones are taking more time?

I haven't done any testing on meaningful workloads yet, but the following
show up on an empty cluster: "creating dump of database schemas",
"analyzing all rows in the new cluster", "setting next transaction ID and
epoch for new cluster", "restoring datbase schemas in the new cluster", and
"sync data directory to disk". I imagine the dump, restore, and
file-copying steps will stand out once you start adding data.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com