Re: Checksum errors in pg_stat_database

On Sat, Mar 9, 2019 at 12:33 AM Julien Rouhaud <rjuju123(at)gmail(dot)com> wrote:

> On Sat, Mar 9, 2019 at 12:35 AM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> >
> > On Mon, Mar 4, 2019 at 11:31 AM Julien Rouhaud <rjuju123(at)gmail(dot)com>
> wrote:
> >>
> >> On Fri, Feb 22, 2019 at 3:01 PM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> >> >
> >> > It tracks things that happen in the general backends. Possibly we
> should also consider counting the errors actually found when running base
> backups? OTOH, that part of the code doesn't really track things like
> databases (as it operates just on the raw data directory underneath), so
> that implementation would definitely not be as clean...
> >>
> >> Sorry I just realized that I totally forgot this part of the thread.
> >>
> >> While it's true that we operate on raw directory, I see that sendDir()
> >> already setup a isDbDir var, and if this is true lastDir should
> >> contain the oid of the underlying database. Wouldn't it be enough to
> >> call sendFile() using this, something like (untested):
> >>
> >> if (!sizeonly)
> >> - sent = sendFile(pathbuf, pathbuf + basepathlen + 1, &statbuf, true);
> >> + sent = sendFile(pathbuf, pathbuf + basepathlen + 1, &statbuf, true,
> >> isDbDir ? pg_atoi(lastDir+1, 4) : InvalidOid);
> >>
> >> and accordingly report any checksum error from sendFile()?
> >
> >
> > That seems it was easy enough. PFA an updated patch that does this, and
> also rebased so it doesn't conflict on oid.
> >
> > (yes, while moving this from draft to publish after lunch, I realized
> that you put a patch togerher for about the same. So let's merge it)
>
> Thanks! Our implementations are quite similar, so I'm fine with most
> of the changes :) I'm just not sure about having two distinct
> functions for reporting failures, given that there's only one caller
> for each. On the other hand it avoids to include miscadmin.h in
> bufpage.c.
>

Yeah, and it brings "cosistence" to at least the calling point(s) within
regular backends.

> That's just a detail, so I'm marking it (again) as ready for committer!
>

Thanks, and pushed :)

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>