|From:||Michael Banck <michael(dot)banck(at)credativ(dot)de>|
|To:||Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, michael(at)paquier(dot)xyz|
|Cc:||coelho(at)cri(dot)ensmp(dot)fr, alvherre(at)2ndquadrant(dot)com, mailings(at)oopsware(dot)de, thomas(dot)munro(at)enterprisedb(dot)com, pgsql-hackers(at)postgresql(dot)org|
|Subject:||Re: Progress reporting for pg_verify_checksums|
|Views:||Raw Message | Whole Thread | Download mbox|
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(at)paquier(dot)xyz> wrote in <20190313072515(dot)GB2988(at)paquier(dot)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.
> > 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
> If the all of aboves are involved, the line would look as the
> [======================= ] ( 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.
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
credativ GmbH, HRB Mönchengladbach 12080
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
|Next Message||Stephen Frost||2019-03-18 21:50:36||Re: Online verification of checksums|
|Previous Message||Peter J. Holzer||2019-03-18 21:19:23||Re: Facing issue in using special characters|