From: | Michael Banck <michael(dot)banck(at)credativ(dot)de> |
---|---|
To: | David Steele <david(at)pgmasters(dot)net>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Magnus Hagander <magnus(at)hagander(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: [PATCH] Verify Checksums during Basebackups |
Date: | 2018-03-23 16:43:24 |
Message-ID: | 1521823404.15036.25.camel@credativ.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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 functionality. It's not
> > > terrible efficient since it rereads the entire buffer for any block
> > > error. A better way is to keep a bitmap for each block in the buffer,
> > > then on retry compare bitmaps. If the block is still bad, report it.
> > > If the block was corrected moved on. If a block was good before but is
> > > bad on retry it can be ignored.
> >
> > I have to admit I find it a bit convoluted and non-obvious on first
> > reading, but I'll try to check it out some more.
>
> Yeah, I think I was influenced too much by how pgBackRest does things,
> which doesn't work as well here. Attached is a simpler version.
This looks much cleaner to me, yeah.
> > I agree that major corruption could make the whole output blow up but I
> > would prefer to keep this feature simple for now, which implies possibly
> > printing out a lot of WARNING or maybe just stopping after the first
> > one (or first few, dunno).
>
> 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 transposed files, rogue files, etc. These produce a lot
> of output.
>
> Maybe stop after five?
I'm on board with this, but I have the feeling that this is not a very
common pattern in Postgres, or might not be project style at all. I
can't remember even seen an error message like that.
Anybody know whether we're doing this in a similar fashion elsewhere?
Michael
--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael(dot)banck(at)credativ(dot)de
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
From | Date | Subject | |
---|---|---|---|
Next Message | David Steele | 2018-03-23 16:54:44 | Re: PATCH: Exclude temp relations from base backup |
Previous Message | David Steele | 2018-03-23 16:26:47 | Re: PATCH: Configurable file mode mask |