Re: pgsql: Add TAP tests for pg_verify_checksums

On 10/13/2018 04:30 AM, Michael Paquier wrote:
> 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?

I have disabled the test temporarily on my two animals since I want to
make sure they are working OK with other changes, and we know what the
problem is. Andres might want to do that with his animal also just add
"--skip-steps=pg_verify_checksums-check" to the command line.

If you want to throw what you have for 3) over to wall to me I can see
if I can finish it.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services