Online verification of checksums

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
180 messages Options
12345 ... 9
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
Hi,

Am Montag, den 17.09.2018, 14:09 -0400 schrieb Stephen Frost:
> > 5. There seems to be no consensus on whether the number of skipped pages
> > should be summarized at the end.
>
> I agree with printing the number of skipped pages, that does seem like
> a nice to have.  I don’t know that actually printing the pages
> themselves is all that useful though. 

Oh ok - I never intended to print out the block numbers themselves, just
the final number of skipped blocks in the summary. So I guess that's
fine and I will add that in my branch.


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
In reply to this post by Stephen Frost
Hi.

Am Montag, den 17.09.2018, 20:45 -0400 schrieb Stephen Frost:

> > You're right it's not about the fsync, sorry for the confusion. My point
> > is that using the checkpoint LSN gives us a guarantee that write is no
> > longer in progress, and so we can't see a page torn because of it. And
> > if we see a partial write due to a new write, it's guaranteed to update
> > the page LSN (and we'll notice it).
>
> Right, no worries about the confusion, I hadn't been fully thinking
> through the LSN bit either and that what we really need is some external
> confirmation of a write having *completed* (not just started) and that
> makes a definite difference.
>
> > > Right, I'm in agreement with doing that and it's what is done in
> > > pgbasebackup and pgBackRest.
> >
> > OK. All I'm saying is pg_verify_checksums should probably do the same
> > thing, i.e. grab checkpoint LSN and roll with that.
>
> Agreed.
I've attached the patch I added to my branch to swap out the pg_sleep()
with a check against the checkpoint LSN on a recheck verification
failure.

Let me know if there are still issues with it. I'll send a new patch for
the whole online verification feature in a bit.


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

pg_verify_checksums_recheck_lsn.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
In reply to this post by Michael Banck-2
Hi,

please find attached version 2 of the patch.

Am Donnerstag, den 26.07.2018, 13:59 +0200 schrieb Michael Banck:

> I've now forward-ported this change to pg_verify_checksums, in order to
> make this application useful for online clusters, see attached patch.
>
> I've tested this in a tight loop (while true; do pg_verify_checksums -D
> data1 -d > /dev/null || /bin/true; done)[2] while doing "while true; do
> createdb pgbench; pgbench -i -s 10 pgbench > /dev/null; dropdb pgbench;
> done", which I already used to develop the original code in the fork and
> which brought up a few bugs.
>
> I got one checksums verification failure this way, all others were
> caught by the recheck (I've introduced a 500ms delay for the first ten
> failures) like this:
>
> > pg_verify_checksums: checksum verification failed on first attempt in
> > file "data1/base/16837/16850", block 7770: calculated checksum 785 but
> > expected 5063
> > pg_verify_checksums: block 7770 in file "data1/base/16837/16850"
> > verified ok on recheck
I have now changed this from the pg_sleep() to a check against the
checkpoint LSN as discussed upthread.

> However, I am also seeing sporadic (maybe 0.5 times per pgbench run)
> failures like this:
>
> > pg_verify_checksums: short read of block 2644 in file
> > "data1/base/16637/16650", got only 4096 bytes
>
> This is not strictly a verification failure, should we do anything about
> this? In my fork, I am also rechecking on this[3] (and I am happy to
> extend the patch that way), but that makes the code and the patch more
> complicated and I wanted to check the general opinion on this case
> first.
I have added a retry for this as well now, without a pg_sleep() as well.
This catches around 80% of the half-reads, but a few slip through. At
that point we bail out with exit(1), and the user can try again, which I
think is fine? 

Alternatively, we could just skip to the next file then and don't make
it count as a checksum failure.

Other changes from V1:

1. Rebased to 422952ee
2. Ignore ENOENT failure during file open and skip to next file
3. Mention total number of skipped blocks during the summary at the end
of the run
4. Skip files starting with pg_internal.init*


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

online-verification-of-checksums_V2.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Stephen Frost
Greetings,

* Michael Banck ([hidden email]) wrote:

> please find attached version 2 of the patch.
>
> Am Donnerstag, den 26.07.2018, 13:59 +0200 schrieb Michael Banck:
> > I've now forward-ported this change to pg_verify_checksums, in order to
> > make this application useful for online clusters, see attached patch.
> >
> > I've tested this in a tight loop (while true; do pg_verify_checksums -D
> > data1 -d > /dev/null || /bin/true; done)[2] while doing "while true; do
> > createdb pgbench; pgbench -i -s 10 pgbench > /dev/null; dropdb pgbench;
> > done", which I already used to develop the original code in the fork and
> > which brought up a few bugs.
> >
> > I got one checksums verification failure this way, all others were
> > caught by the recheck (I've introduced a 500ms delay for the first ten
> > failures) like this:
> >
> > > pg_verify_checksums: checksum verification failed on first attempt in
> > > file "data1/base/16837/16850", block 7770: calculated checksum 785 but
> > > expected 5063
> > > pg_verify_checksums: block 7770 in file "data1/base/16837/16850"
> > > verified ok on recheck
>
> I have now changed this from the pg_sleep() to a check against the
> checkpoint LSN as discussed upthread.
Ok.

> > However, I am also seeing sporadic (maybe 0.5 times per pgbench run)
> > failures like this:
> >
> > > pg_verify_checksums: short read of block 2644 in file
> > > "data1/base/16637/16650", got only 4096 bytes
> >
> > This is not strictly a verification failure, should we do anything about
> > this? In my fork, I am also rechecking on this[3] (and I am happy to
> > extend the patch that way), but that makes the code and the patch more
> > complicated and I wanted to check the general opinion on this case
> > first.
>
> I have added a retry for this as well now, without a pg_sleep() as well.

> This catches around 80% of the half-reads, but a few slip through. At
> that point we bail out with exit(1), and the user can try again, which I
> think is fine? 

No, this is perfectly normal behavior, as is having completely blank
pages, now that I think about it.  If we get a short read then I'd say
we simply check that we got an EOF and, in that case, we just move on.

> Alternatively, we could just skip to the next file then and don't make
> it count as a checksum failure.

No, I wouldn't count it as a checksum failure.  We could possibly count
it towards the skipped pages, though I'm even on the fence about that.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

David Steele
On 9/18/18 11:45 AM, Stephen Frost wrote:
> * Michael Banck ([hidden email]) wrote:

>> I have added a retry for this as well now, without a pg_sleep() as well.
>
>> This catches around 80% of the half-reads, but a few slip through. At
>> that point we bail out with exit(1), and the user can try again, which I
>> think is fine? 
>
> No, this is perfectly normal behavior, as is having completely blank
> pages, now that I think about it.  If we get a short read then I'd say
> we simply check that we got an EOF and, in that case, we just move on.
>
>> Alternatively, we could just skip to the next file then and don't make
>> it count as a checksum failure.
>
> No, I wouldn't count it as a checksum failure.  We could possibly count
> it towards the skipped pages, though I'm even on the fence about that.
+1 for it not being a failure.  Personally I'd count it as a skipped
page, since we know the page exists but it can't be verified.

The other option is to wait for the page to stabilize, which doesn't
seem like it would take very long in most cases -- unless you are doing
this test from another host with shared storage.  Then I would expect to
see all kinds of interesting torn pages after the last checkpoint.

Regards,
--
-David
[hidden email]


signature.asc (891 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
Hi,

Am Dienstag, den 18.09.2018, 13:52 -0400 schrieb David Steele:

> On 9/18/18 11:45 AM, Stephen Frost wrote:
> > * Michael Banck ([hidden email]) wrote:
> > > I have added a retry for this as well now, without a pg_sleep() as well.
> > > This catches around 80% of the half-reads, but a few slip through. At
> > > that point we bail out with exit(1), and the user can try again, which I
> > > think is fine? 
> >
> > No, this is perfectly normal behavior, as is having completely blank
> > pages, now that I think about it.  If we get a short read then I'd say
> > we simply check that we got an EOF and, in that case, we just move on.
> >
> > > Alternatively, we could just skip to the next file then and don't make
> > > it count as a checksum failure.
> >
> > No, I wouldn't count it as a checksum failure.  We could possibly count
> > it towards the skipped pages, though I'm even on the fence about that.
>
> +1 for it not being a failure.  Personally I'd count it as a skipped
> page, since we know the page exists but it can't be verified.
>
> The other option is to wait for the page to stabilize, which doesn't
> seem like it would take very long in most cases -- unless you are doing
> this test from another host with shared storage.  Then I would expect to
> see all kinds of interesting torn pages after the last checkpoint.
OK, I'm skipping the block now on first try, as this makes (i) sense and
(ii) simplifies the code (again).

Version 3 is attached.


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

online-verification-of-checksums_V3.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Fabien COELHO-3

Hallo Michael,

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.

The patch is missing a documentation update.

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?

   + if (block_retry == false)

Why not trust boolean operations?

   if (!block_retry)

--
Fabien.

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
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: [hidden email]

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

online-verification-of-checksums_V4.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Stephen Frost
Greetings,

* Michael Banck ([hidden email]) wrote:

> Am Mittwoch, den 26.09.2018, 13:23 +0200 schrieb Fabien COELHO:
> > 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?
I certainly don't see a lot of point in doing much more than what was
discussed previously for 'new' blocks (counting them as skipped and
moving on).

An actual read() error (that is, a failure on a read() call such as
getting back EIO), on the other hand, is something which I'd probably
report back to the user immediately and then move on, and perhaps
report again at the end.

Note that a short read isn't an error and falls under the 'new' blocks
discussion above.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Fabien COELHO-3
In reply to this post by Michael Banck-2

>> 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?

Yes, and also checking that the described behavior correspond to the new
version.

>> 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.

Hmmm.

I'm really saying that it is debatable, so here is some fuel to the
debate:

If I run the check command and it cannot do its job, there is a problem
which is as bad as a failing checksum. The only safe assumption on a
cannot-read block is that the checksum is bad... So ISTM that on
on some of the "skipped" errors there should be appropriate report (exit
code, final output) that something is amiss.

--
Fabien.

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
In reply to this post by Stephen Frost
Hi,

Am Mittwoch, den 26.09.2018, 10:54 -0400 schrieb Stephen Frost:

> * Michael Banck ([hidden email]) wrote:
> > Am Mittwoch, den 26.09.2018, 13:23 +0200 schrieb Fabien COELHO:
> > > 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?
>
> I certainly don't see a lot of point in doing much more than what was
> discussed previously for 'new' blocks (counting them as skipped and
> moving on).
>
> An actual read() error (that is, a failure on a read() call such as
> getting back EIO), on the other hand, is something which I'd probably
> report back to the user immediately and then move on, and perhaps
> report again at the end.
>
> Note that a short read isn't an error and falls under the 'new' blocks
> discussion above.
So I've added ENOENT checks when opening or statting files, i.e. EIO
would still be reported.

The current code in master exits on reads which do not return BLCKSZ,
which I've changed to a skip. So that means we now no longer check for
read failures (return code < 0) so I have now added a check for that and
emit an error message and return.

New version 5 attached.


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

online-verification-of-checksums_V5.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Fabien COELHO-3
In reply to this post by Stephen Frost

Hello Stephen,

> I certainly don't see a lot of point in doing much more than what was
> discussed previously for 'new' blocks (counting them as skipped and
> moving on).

Sure.

> An actual read() error (that is, a failure on a read() call such as
> getting back EIO), on the other hand, is something which I'd probably
> report back to the user immediately and then move on, and perhaps
> report again at the end.

Yep.

> Note that a short read isn't an error and falls under the 'new' blocks
> discussion above.

I'm really unsure that a short read should really be coldly skipped:

If the check is offline, then one file is in a very bad state, this is
really a panic situation.

If the check is online, given that both postgres and the verify command
interact with the same OS (?) and at the pg page level, I'm not sure in
which situation there could be a partial block, because pg would only
send full pages to the OS.

--
Fabien.

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Stephen Frost
Greetings,

* Fabien COELHO ([hidden email]) wrote:
> >Note that a short read isn't an error and falls under the 'new' blocks
> >discussion above.
>
> I'm really unsure that a short read should really be coldly skipped:
>
> If the check is offline, then one file is in a very bad state, this is
> really a panic situation.

Why?  Are we sure that's really something which can't ever happen, even
if the database was shutdown with 'immediate'?  I don't think it can but
that's something to consider.  In any case, my comments were
specifically thinking about it from an 'online' perspective.

> If the check is online, given that both postgres and the verify command
> interact with the same OS (?) and at the pg page level, I'm not sure in
> which situation there could be a partial block, because pg would only send
> full pages to the OS.

The OS doesn't operate at the same level that PG does- a single write in
PG could get blocked and scheduled off after having only copied half of
the 8k that PG sends.  This isn't really debatable- we've seen it happen
and everything is operating perfectly correctly, it just happens that
you were able to get a read() at the same time a write() was happening
and that only part of the page had been updated at that point.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Tomas Vondra-4
In reply to this post by Michael Banck-2
Hi,

On 09/26/2018 05:15 PM, Michael Banck wrote:
> ...
>
> New version 5 attached.
>

I've looked at v5, and the retry/recheck logic seems OK to me - I'd
still vote to keep it consistent with what pg_basebackup does (i.e.
doing the LSN check first, before looking at the checksum), but I don't
think it's a bug.

I'm not sure about the other issues brought up (ENOENT, short reads). I
haven't given it much thought.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Tomas Vondra-4
In reply to this post by Michael Banck-2
Hi,

One more thought - when running similar tools on a live system, it's
usually a good idea to limit the impact by throttling the throughput. As
the verification runs in an independent process it can't reuse the
vacuum-like cost limit directly, but perhaps it could do something
similar? Like, limit the number of blocks read/second, or so?

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Paquier-2
On Sat, Sep 29, 2018 at 10:51:23AM +0200, Tomas Vondra wrote:
> One more thought - when running similar tools on a live system, it's
> usually a good idea to limit the impact by throttling the throughput. As
> the verification runs in an independent process it can't reuse the
> vacuum-like cost limit directly, but perhaps it could do something
> similar? Like, limit the number of blocks read/second, or so?

When it comes to such parameters, not using a number of blocks but
throttling with a value in bytes (kB or MB of course) speaks more to the
user.  The past experience with checkpoint_segments is one example of
that.  Converting that to a number of blocks internally would definitely
make sense the most sense.  +1 for this idea.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Stephen Frost
Greetings,

* Michael Paquier ([hidden email]) wrote:

> On Sat, Sep 29, 2018 at 10:51:23AM +0200, Tomas Vondra wrote:
> > One more thought - when running similar tools on a live system, it's
> > usually a good idea to limit the impact by throttling the throughput. As
> > the verification runs in an independent process it can't reuse the
> > vacuum-like cost limit directly, but perhaps it could do something
> > similar? Like, limit the number of blocks read/second, or so?
>
> When it comes to such parameters, not using a number of blocks but
> throttling with a value in bytes (kB or MB of course) speaks more to the
> user.  The past experience with checkpoint_segments is one example of
> that.  Converting that to a number of blocks internally would definitely
> make sense the most sense.  +1 for this idea.
While I agree this would be a nice additional feature to have, it seems
like something which could certainly be added later and doesn't
necessairly have to be included in the initial patch.  If Michael has
time to add that, great, if not, I'd rather have this as-is than not.

I do tend to agree with Michael that having the parameter be specified
as (or at least able to accept) a byte-based value is a good idea.  As
another feature idea, having this able to work in parallel across
tablespaces would be nice too.  I can certainly imagine some point where
this is a default process which scans the database at a slow pace across
all the tablespaces more-or-less all the time checking for corruption.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Tomas Vondra-4


On 09/29/2018 02:14 PM, Stephen Frost wrote:

> Greetings,
>
> * Michael Paquier ([hidden email]) wrote:
>> On Sat, Sep 29, 2018 at 10:51:23AM +0200, Tomas Vondra wrote:
>>> One more thought - when running similar tools on a live system, it's
>>> usually a good idea to limit the impact by throttling the throughput. As
>>> the verification runs in an independent process it can't reuse the
>>> vacuum-like cost limit directly, but perhaps it could do something
>>> similar? Like, limit the number of blocks read/second, or so?
>>
>> When it comes to such parameters, not using a number of blocks but
>> throttling with a value in bytes (kB or MB of course) speaks more to the
>> user.  The past experience with checkpoint_segments is one example of
>> that.  Converting that to a number of blocks internally would definitely
>> make sense the most sense.  +1 for this idea.
>
> While I agree this would be a nice additional feature to have, it seems
> like something which could certainly be added later and doesn't
> necessairly have to be included in the initial patch.  If Michael has
> time to add that, great, if not, I'd rather have this as-is than not.
>

True, although I don't think it'd be particularly difficult.

> I do tend to agree with Michael that having the parameter be specified
> as (or at least able to accept) a byte-based value is a good idea.

Sure, I was not really expecting it to be exposed as raw block count. I
agree it should be in byte-based values (i.e. just like --max-rate in
pg_basebackup).

> As another feature idea, having this able to work in parallel across
> tablespaces would be nice too.  I can certainly imagine some point where
> this is a default process which scans the database at a slow pace across
> all the tablespaces more-or-less all the time checking for corruption.
>

Maybe, but that's certainly a non-trivial feature.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Fabien COELHO-3
In reply to this post by Michael Banck-2

Hallo Michael,

> New version 5 attached.

Patch does not seem to apply anymore.

Moreover, ISTM that some discussions about behavioral changes are not
fully settled.

My current opinion is that when offline some errors are not admissible,
whereas the same errors are admissible when online because they may be due
to the ongoing database processing, so the behavior should not be strictly
the same.

This might suggest some option to tell the command that it should work in
online or offline mode, so that it may be stricter in some cases. The
default may be one of the option, eg the stricter offline mode, or maybe
guessed at startup.

I put the patch in "waiting on author" state.

--
Fabien.

Reply | Threaded
Open this post in threaded view
|

Re: Online verification of checksums

Michael Banck-2
Hi Fabien,

On Thu, Oct 25, 2018 at 10:16:03AM +0200, Fabien COELHO wrote:
> >New version 5 attached.
>
> Patch does not seem to apply anymore.

Thanks, rebased version attached.

> Moreover, ISTM that some discussions about behavioral changes are not fully
> settled.
>
> My current opinion is that when offline some errors are not admissible,
> whereas the same errors are admissible when online because they may be due
> to the ongoing database processing, so the behavior should not be strictly
> the same.

Indeed, the recently-added pg_verify_checksums testsuite adds a few
files with just 'foo' in them and with V5 of the patch,
pg_verify_checksums no longer bails out with an error on those.

I have now re-added the retry logic for partially-read pages, so that it
bails out if it reads a page partially twice. This makes the testsuite
work again.

I am not convinced we need to differentiate further between online and
offline operation, can you explain in more detail which other
differences are ok in online mode and why?
 
> This might suggest some option to tell the command that it should work in
> online or offline mode, so that it may be stricter in some cases. The
> default may be one of the option, eg the stricter offline mode, or maybe
> guessed at startup.

If we believe the operation should be different, the patch removes the
"is cluster online?" check (as it is no longer necessary), so we could
just replace the current error message with a global variable with the
result of that check and use it where needed (if any).


Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: [hidden email]

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

online-verification-of-checksums_V6.patch (7K) Download Attachment
12345 ... 9