Re: pgsql: Add TAP tests for pg_verify_checksums

On Fri, Oct 12, 2018 at 12:14:48PM +0200, Michael Banck wrote:
> On Fri, Oct 12, 2018 at 03:05:43PM +0900, Michael Paquier wrote:
>> On Fri, Oct 12, 2018 at 12:11:58PM +0900, Michael Paquier wrote:
>>> Agreed. I am just working on a patch for v11- which uses a
>>> whitelist-based method instead of what is present now. Reverting the
>>> tests to put the buildfarm to green could be done, but that's not the
>>> root of the problem.
>
> I think that's the right solution; the online verification patch adds
> even more logic to the blacklist so getting rid of it in favor of a
> whitelist is +1 with me.

Thanks Michael for the input!

>> So, I have coded this thing, and finish with the attached. The
>> following file patterns are accepted for the checksums:
>> <digits>.<segment>
>> <digits>_<forkname>
>> <digits>_<forkname>.<segment>
>> I have added some tests on the way to make sure that all the patterns
>> get covered. Please note that this skips the temporary files.
>
> Maybe also add some correct (to be scanned) but non-empty garbage files
> later on that it should barf on?

I was not sure about doing that in the main patch so I tweaked manually
the test to make sure that the tool still complained with "could not
read block" as it should. That's easy enough to add, so I'll add them
with multiple file patterns. Those are cheap checks as well if they are
placed in global/.

Another problem that the patch has is that it is not using
forkname_to_number() to scan for all the fork types, and I forgot init
forks in the previous version. Using forkname_to_number() also makes
the tool more bug-proof, it is also not complicated to plug into the
existing patch.

Anyway, I have a bit of a problem here, which prevents me to stay in
front of a computer or to look at a screen for more than a couple of
minutes in a row for a couple of days at least, and I don't like to keep
the buildfarm unhappy for the time being. There are three options:
1) Revert the TAP tests of pg_verify_checksums.
2) Push the patch which adds new entries for EXEC_BACKEND files in the
skip list. That's a short workaround, and that would allow default
deployments of Postgres to use the tool.
3) Finish the patch with the whitelist approach.

I can do 1) or 2) in my condition. 3) requires more work than I can do
now, though the patch to do is getting in shape, so the buildfarm would
stay unhappy. Any preference of the course of action to take?
--
Michael