I still think that the speed should compute a double to avoid integer
rounding errors within the computation. ISTM that rounding should only be
done for display in the end.
Ok, also done this way. I decided to print only full MB and not any
further digits as those don't seem very relevant.
For the computation to be in double, you must convert to double somewhere
in the formula, otherwise it is computed as ints and only cast as a double
afterwards. Maybe:
current_speed = (1000.0 / MEGABYTES) * current_size / elapsed;
Or anything which converts to double early.
Instead of casting percent to int, maybe use "%.0f" as well, just like
current_speed?
+ if ((blockno % 100 == 0) && show_progress)
I'd invert the condition to avoid a modulo when not needed.
I'm not very found of global variables, but it does not matter here and
doing it differently would involve more code. It would matter though if
the progress report would be shared between commands.
I'm okay with calling the report on each file even if this means every few
GB...
For my I/O throttling branch I've backed off to only call the report
every 100 blocks (and at the end of the file). I've added that logic to
this patch as well.
The 100 blocks = 800 KiB looks arbitrary. What is the rational? ISTM that
postgres will tend to store a power of two number of pages on full
segments, so maybe there could be a rational for 1024. Not sure.
Someone suggested ETA, and it seems rather simple to do. What about
adding it?
I don't know, just computing the ETA is easy of course. But you'd have
to fiddle with seconds/minutes/hours conversions cause the runtime could
be hours. Then, you don't want to have it bumping around weirdly when
your I/O rate changes, cf. https://xkcd.com/612/ That doesn't sounds
simpler than the signal trigger we might get rid of.
Ok, it is more complicated that it looks for large sizes if second is not
the right display unit.
--
Fabien.
