Re: Progress reporting for pg_verify_checksums
От | Michael Banck |
---|---|
Тема | Re: Progress reporting for pg_verify_checksums |
Дата | |
Msg-id | 1552944782.9697.45.camel@credativ.de обсуждение исходный текст |
Ответ на | Re: Progress reporting for pg_verify_checksums (Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>) |
Ответы |
Re: Progress reporting for pg_verify_checksums
(Fabien COELHO <coelho@cri.ensmp.fr>)
|
Список | pgsql-hackers |
Hi, thanks for the additional review! Am Donnerstag, den 14.03.2019, 11:54 +0900 schrieb Kyotaro HORIGUCHI: > At Wed, 13 Mar 2019 16:25:15 +0900, Michael Paquier <michael@paquier.xyz> wrote in <20190313072515.GB2988@paquier.xyz> > > On Wed, Mar 13, 2019 at 07:22:28AM +0100, Fabien COELHO wrote: > > > Does not apply because of the renaming committed by Michaël. > > > > > > Could you rebase? > > > > This stuff touches pg_checksums.c, so you may want to wait one day or > > two to avoid extra work... I think that I'll be able to finish the > > addition of the --enable and --disable switches by then. I have rebased it now. > > + /* Make sure we report at most once every tenth of a second */ > > + if ((INSTR_TIME_GET_MILLISEC(now) - > > + INSTR_TIME_GET_MILLISEC(last_progress_update) < 100) && !force) > > I'm not a skilled gamer and 10Hz update is a bit hard for my > eyes:p The second hand of old clocks ticks at 4Hz. I think it is > enough frequently. Ok. > > 841/1516 MB (55%, 700 MB/s) > > Differently from other prgress reporting project, estimating ETC > (estimated time to completion), which is in the most important, > is quite easy. (pgbench does that.) And I'd like to see a > progress bar there but it might be overdoing. I'm not sure let > the current size occupy a part of screen width is needed. I > don't think total size is needed to be fixed in MB and I think at > most four or five digits are enough. (That is the same for > speed.) > > If the all of aboves are involved, the line would look as the > follows. > > [======================= ] ( 63% of 12.53 GB, 179 MB/s, ETC 26s) > > # Note that this is just an opinion. > > (pg_checksum runs fast at the beginning so ETC behaves somewhat > strange in the meanwhile.) I haven't changed that for now as it seems to be a bit more involved. I'd like to hear other opinions on whether that is worthwhile? > > +#define MEGABYTES 1048576 > > 1024 * 1024 is preferable than that many digits. Good point, changed that way. > > + /* we handle SIGUSR1 only, and toggle the value of show_progress */ > > + if (signum == SIGUSR1) > > + show_progress = !show_progress; > > SIGUSR1 *toggles* progress. Not sure what you mean here, are you saying it should be called toggle_progress and not show_progress? I think it was like that originally but got changed due to review comments. > A garbage line is left alone after turning it off. It would be > erasable. I'm not sure which is better, though. How about just printing a "\n" in the signal handler if we are in a terminal? I think erasing the line might get too clever and will be a portability hazard? I have done that now in the attached patch, let me know what you think. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.banck@credativ.de credativ GmbH, HRB Mönchengladbach 12080 USt-ID-Nummer: DE204566209 Trompeterallee 108, 41189 Mönchengladbach Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer Unser Umgang mit personenbezogenen Daten unterliegt folgenden Bestimmungen: https://www.credativ.de/datenschutz
Вложения
В списке pgsql-hackers по дате отправления: