Hello.
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.
> + /* 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
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.)
> +#define MEGABYTES 1048576
1024 * 1024 is preferable than that many digits.
> + /* we handle SIGUSR1 only, and toggle the value of show_progress */
> + if (signum == SIGUSR1)
> + show_progress = !show_progress;
SIGUSR1 *toggles* progress. A garbage line is left alone after
turning it off. It would be erasable. I'm not sure which is
better, though.
>
> @@ -167,7 +255,7 @@ scan_directory(const char *basedir, const char *subdir)
> if (strncmp(de->d_name,
> PG_TEMP_FILES_DIR,
> strlen(PG_TEMP_FILES_DIR)) == 0)
> - return;
> + continue;
Why this patch changes the behavior for temprary directories? It
seems like a bug fix of pg_checksums.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
