Hi,
Am Mittwoch, den 26.09.2018, 13:23 +0200 schrieb Fabien COELHO:
> Patch v3 applies cleanly, code compiles and make check is ok, but the
> command is probably not tested anywhere, as already mentioned on other
> threads.
Right.
> The patch is missing a documentation update.
I've added that now. I think the only change needed was removing the
"server needs to be offline" part?
> There are debatable changes of behavior:
>
> if (errno == ENOENT) return / continue...
>
> For instance, a file disappearing is ok online, but not so if offline. On
> the other hand, the probability that a file suddenly disappears while the
> server offline looks remote, so reporting such issues does not seem
> useful.
>
> However I'm more wary with other continues/skips added. ISTM that skipping
> a block because of a read error, or because it is new, or some other
> reasons, is not the same thing, so should be counted & reported
> differently?
I think that would complicate things further without a lot of benefit.
After all, we are interested in checksum failures, not necessarily read
failures etc. so exiting on them (and skip checking possibly large parts
of PGDATA) looks undesirable to me.
So I have done no changes in this part so far, what do others think
about this?
> + if (block_retry == false)
>
> Why not trust boolean operations?
>
> if (!block_retry)
I've changed that as well.
Version 4 is attached.
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