On 10/13/2018 10:00 AM, Andrew Dunstan wrote:
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.
It occurred to me that a pretty simple fix could just be to blacklist
everything that didn't start with a digit. The whitelist approach is
probably preferable ... depends how urgent we see this as.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
