Re: [PATCH] Verify Checksums during Basebackups

On Tue, Apr 3, 2018 at 4:32 PM, Tom Lane wrote: > I wrote: > > Magnus Hagander writes: > >> Unless.. %ld is the wrong thing to print: > >> static int64 total_checksum_failures; > >> We should perhaps be using something other than %ld to print that? > > >

Re: [PATCH] Verify Checksums during Basebackups

I wrote: > Magnus Hagander writes: >> Unless.. %ld is the wrong thing to print: >> static int64 total_checksum_failures; >> We should perhaps be using something other than %ld to print that? > INT64_FORMAT. BTW, don't just stick INT64_FORMAT into the

Re: [PATCH] Verify Checksums during Basebackups

Magnus Hagander writes: > Unless.. %ld is the wrong thing to print: > static int64 total_checksum_failures; > We should perhaps be using something other than %ld to print that? INT64_FORMAT. regards, tom lane

Re: [PATCH] Verify Checksums during Basebackups

On Tue, Apr 3, 2018 at 4:25 PM, Tom Lane wrote: > Magnus Hagander writes: > > Seems the tests are failing on prairiedog: > > https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl? > nm=prairiedog=2018-04-03%2012%3A15%3A27=pg_basebackup-check > > I

Re: [PATCH] Verify Checksums during Basebackups

Magnus Hagander writes: > Seems the tests are failing on prairiedog: > https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=prairiedog=2018-04-03%2012%3A15%3A27=pg_basebackup-check > I don't have time to dive in right now, but that seems interesting -- it's >

Re: [PATCH] Verify Checksums during Basebackups

On Tue, Apr 3, 2018 at 4:00 PM, Michael Banck wrote: > Hi Magnus, > > Am 03.04.2018 13:59 schrieb Magnus Hagander : > > And of course I forgot that particular part in the first push, so I've > pushed it as a separate commit. > > > Many thanks for

Re: [PATCH] Verify Checksums during Basebackups

Hi Magnus,Am 03.04.2018 13:59 schrieb Magnus Hagander :And of course I forgot that particular part in the first push, so I've pushed it as a separate commit. Many thanks for cleaning up the patch and committing it!Michael-- Michael BanckProjektleiter / Senior BeraterTel.: +49

Re: [PATCH] Verify Checksums during Basebackups

On Tue, Apr 3, 2018 at 1:52 PM, Magnus Hagander wrote: > > > On Mon, Apr 2, 2018 at 2:48 PM, Michael Banck > wrote: > >> Hi! >> >> On Sun, Apr 01, 2018 at 05:27:21PM +0200, Magnus Hagander wrote: >> > It might be a micro-optimisation, but ISTM

Re: [PATCH] Verify Checksums during Basebackups

On Mon, Apr 2, 2018 at 2:48 PM, Michael Banck wrote: > Hi! > > On Sun, Apr 01, 2018 at 05:27:21PM +0200, Magnus Hagander wrote: > > It might be a micro-optimisation, but ISTM that we shouldn't do the > > basename(palloc(fn)) in is_heap_file() unless we actually need it

Re: [PATCH] Verify Checksums during Basebackups

Hi! On Sun, Apr 01, 2018 at 05:27:21PM +0200, Magnus Hagander wrote: > It might be a micro-optimisation, but ISTM that we shouldn't do the > basename(palloc(fn)) in is_heap_file() unless we actually need it -- so not > at the top of the function. (And surely "." and ".." should not occur here? >

Re: [PATCH] Verify Checksums during Basebackups

On Sat, Mar 31, 2018 at 2:54 PM, Michael Banck wrote: > Hi, > > On Fri, Mar 30, 2018 at 07:46:02AM -0400, Stephen Frost wrote: > > * Magnus Hagander (mag...@hagander.net) wrote: > > > On Fri, Mar 30, 2018 at 5:35 AM, David Steele > wrote: > > > >

Re: [PATCH] Verify Checksums during Basebackups

Hi, On Fri, Mar 30, 2018 at 07:46:02AM -0400, Stephen Frost wrote: > * Magnus Hagander (mag...@hagander.net) wrote: > > On Fri, Mar 30, 2018 at 5:35 AM, David Steele wrote: > > > > > On 3/24/18 10:32 AM, Michael Banck wrote: > > > > Am Freitag, den 23.03.2018, 17:43 +0100

Re: [PATCH] Verify Checksums during Basebackups

Greetings, * Magnus Hagander (mag...@hagander.net) wrote: > On Fri, Mar 30, 2018 at 5:35 AM, David Steele wrote: > > > On 3/24/18 10:32 AM, Michael Banck wrote: > > > Am Freitag, den 23.03.2018, 17:43 +0100 schrieb Michael Banck: > > >> Am Freitag, den 23.03.2018, 10:54

Re: [PATCH] Verify Checksums during Basebackups

On Fri, Mar 30, 2018 at 5:35 AM, David Steele wrote: > On 3/24/18 10:32 AM, Michael Banck wrote: > > Am Freitag, den 23.03.2018, 17:43 +0100 schrieb Michael Banck: > >> Am Freitag, den 23.03.2018, 10:54 -0400 schrieb David Steele: > >>> In my experience actual block errors

Re: [PATCH] Verify Checksums during Basebackups

On 3/24/18 10:32 AM, Michael Banck wrote: > Am Freitag, den 23.03.2018, 17:43 +0100 schrieb Michael Banck: >> Am Freitag, den 23.03.2018, 10:54 -0400 schrieb David Steele: >>> In my experience actual block errors are relatively rare, so there >>> aren't likely to be more than a few in a file.

Re: [PATCH] Verify Checksums during Basebackups

Hi, Am Freitag, den 23.03.2018, 17:43 +0100 schrieb Michael Banck: > Am Freitag, den 23.03.2018, 10:54 -0400 schrieb David Steele: > > In my experience actual block errors are relatively rare, so there > > aren't likely to be more than a few in a file. More common are > > overwritten or

Re: [PATCH] Verify Checksums during Basebackups

Hi David, Am Freitag, den 23.03.2018, 10:54 -0400 schrieb David Steele: > On 3/23/18 5:36 AM, Michael Banck wrote: > > Am Donnerstag, den 22.03.2018, 12:22 -0400 schrieb David Steele: > > > > > > +if (phdr->pd_checksum != checksum) > > > > > > I've attached a patch that adds basic retry

Re: [PATCH] Verify Checksums during Basebackups

Hi Michael, On 3/23/18 5:36 AM, Michael Banck wrote: > Am Donnerstag, den 22.03.2018, 12:22 -0400 schrieb David Steele: >> >> +if (phdr->pd_checksum != checksum) >> >> I've attached a patch that adds basic retry functionality. It's not >> terrible efficient since it rereads the entire buffer

Re: [PATCH] Verify Checksums during Basebackups

Hi David, thanks for the review! Am Donnerstag, den 22.03.2018, 12:22 -0400 schrieb David Steele: > On 3/17/18 5:34 PM, Michael Banck wrote: > > On Fri, Mar 09, 2018 at 10:35:33PM +0100, Michael Banck wrote: > > I think most people (including those I had off-list discussions about > > this with)

Re: [PATCH] Verify Checksums during Basebackups

Hi Magnus, thanks a lot for looking at my patch! Am Donnerstag, den 22.03.2018, 15:07 +0100 schrieb Magnus Hagander: > On Sat, Mar 17, 2018 at 10:34 PM, Michael Banck > wrote: > > On Fri, Mar 09, 2018 at 10:35:33PM +0100, Michael Banck wrote: > > > Possibly open

Re: [PATCH] Verify Checksums during Basebackups

Hi Michael, On 3/17/18 5:34 PM, Michael Banck wrote: > > On Fri, Mar 09, 2018 at 10:35:33PM +0100, Michael Banck wrote: > > I think most people (including those I had off-list discussions about > this with) were of the opinion that such an option should be there, so I > added an additional

Re: [PATCH] Verify Checksums during Basebackups

On Sat, Mar 17, 2018 at 10:34 PM, Michael Banck wrote: > Hi, > > On Fri, Mar 09, 2018 at 10:35:33PM +0100, Michael Banck wrote: > > Possibly open questions: > > > > 1. I have not so far changed the replication protocol to make verifying > > checksums optional. I can go

Re: [PATCH] Verify Checksums during Basebackups

Hi, On Fri, Mar 09, 2018 at 10:35:33PM +0100, Michael Banck wrote: > Possibly open questions: > > 1. I have not so far changed the replication protocol to make verifying > checksums optional. I can go about that next if the consensus is that we > need such an option (and cannot just check it

Re: [PATCH] Verify Checksums during Basebackups

Hi, Am Mittwoch, den 28.02.2018, 19:08 +0100 schrieb Michael Banck: > some installations have data which is only rarerly read, and if they are > so large that dumps are not routinely taken, data corruption would only > be detected with some large delay even with checksums enabled. > > The

Re: [PATCH] Verify Checksums during Basebackups

Hi Michael, On 3/5/18 6:36 AM, Stephen Frost wrote: > * Michael Banck (michael.ba...@credativ.de) wrote: > >> So I guess this would have to be sent back via the replication protocol, >> but I don't see an off-hand way to do this easily? > > The final ordinary result set could be extended to

Re: [PATCH] Verify Checksums during Basebackups

Michael, * Michael Banck (michael.ba...@credativ.de) wrote: > Am Montag, den 05.03.2018, 06:36 -0500 schrieb Stephen Frost: > > * Michael Banck (michael.ba...@credativ.de) wrote: > > > On Sun, Mar 04, 2018 at 06:19:00PM +0100, Magnus Hagander wrote: > > > > So sure, if we go with WARNING + exit

Re: [PATCH] Verify Checksums during Basebackups

Hi, Am Montag, den 05.03.2018, 06:36 -0500 schrieb Stephen Frost: > Michael, > > * Michael Banck (michael.ba...@credativ.de) wrote: > > On Sun, Mar 04, 2018 at 06:19:00PM +0100, Magnus Hagander wrote: > > > So sure, if we go with WARNING + exit with an errorcode, that is perhaps > > > the best

Re: [PATCH] Verify Checksums during Basebackups

Michael, * Michael Banck (michael.ba...@credativ.de) wrote: > On Sun, Mar 04, 2018 at 06:19:00PM +0100, Magnus Hagander wrote: > > So sure, if we go with WARNING + exit with an errorcode, that is perhaps > > the best combination of the two. > > I had a look at how to go about this, but it

Re: [PATCH] Verify Checksums during Basebackups

Hi, On Sun, Mar 04, 2018 at 06:19:00PM +0100, Magnus Hagander wrote: > So sure, if we go with WARNING + exit with an errorcode, that is perhaps > the best combination of the two. I had a look at how to go about this, but it appears to be a bit complicated; the first problem is that sendFile()

Re: [PATCH] Verify Checksums during Basebackups

On Sun, Mar 4, 2018 at 3:49 PM, Stephen Frost wrote: > Greetings Magnus, all, > > * Magnus Hagander (mag...@hagander.net) wrote: > > I think it would also be a good idea to have this a three-mode setting, > > with "no check", "check and warning", "check and error". Where

Re: [PATCH] Verify Checksums during Basebackups

Greetings Magnus, all, * Magnus Hagander (mag...@hagander.net) wrote: > I think it would also be a good idea to have this a three-mode setting, > with "no check", "check and warning", "check and error". Where "check and > error" should be the default, but you could turn off that in "save whatever

Re: [PATCH] Verify Checksums during Basebackups

On Fri, Mar 2, 2018 at 7:04 PM, Robert Haas wrote: > On Fri, Mar 2, 2018 at 6:23 AM, Magnus Hagander > wrote: > > Another quick note -- we need to assert that the size of the buffer is > > actually divisible by BLCKSZ. I don't think it's a common

Re: [PATCH] Verify Checksums during Basebackups

On Fri, Mar 2, 2018 at 6:23 AM, Magnus Hagander wrote: > Another quick note -- we need to assert that the size of the buffer is > actually divisible by BLCKSZ. I don't think it's a common scenario, but it > could break badly if somebody changes BLCKSZ. Either that or perhaps

Re: [PATCH] Verify Checksums during Basebackups

On Wed, Feb 28, 2018 at 7:08 PM, Michael Banck wrote: > Hi, > > some installations have data which is only rarerly read, and if they are > so large that dumps are not routinely taken, data corruption would only > be detected with some large delay even with checksums

Re: [PATCH] Verify Checksums during Basebackups

On 2/28/18 1:08 PM, Michael Banck wrote: > > The attached small patch verifies checksums (in case they are enabled) > during a basebackup. The rationale is that we are reading every block in > this case anyway, so this is a good opportunity to check them as well. > Other and complementary ways of

[PATCH] Verify Checksums during Basebackups

Hi, some installations have data which is only rarerly read, and if they are so large that dumps are not routinely taken, data corruption would only be detected with some large delay even with checksums enabled. The attached small patch verifies checksums (in case they are enabled) during a