On Sat, Mar 9, 2019 at 10:41 AM Julien Rouhaud <rjuju...@gmail.com> wrote:
> On Sat, Mar 9, 2019 at 9:34 AM Julien Rouhaud <rjuju...@gmail.com> wrote: > > > > On Sat, Mar 9, 2019 at 12:35 AM Magnus Hagander <mag...@hagander.net> > wrote: > > > > > > On Mon, Mar 4, 2019 at 11:31 AM Julien Rouhaud <rjuju...@gmail.com> > wrote: > > >> > > >> On Fri, Feb 22, 2019 at 3:01 PM Magnus Hagander <mag...@hagander.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. > > > > > Sorry, I have again new comments after a little bit more thinking. > I'm wondering if we can do something about shared objects while we're > at it. They don't belong to any database, so it's a little bit > orthogonal to this proposal, but it seems quite important to track > error on those too! > > What about adding a new field in PgStat_GlobalStats for that? We can > use the same lastDir to easily detect such objects and slightly adapt > sendFile again, which seems quite straightforward. > Ah, didn't spot that one until after I pushed :/ Sorry about that. Hmm. That's an interesting thought. And then add a column to pg_stat_bgwriter, I assume? (Which is an ever increasingly bad name for the view, but that's unrelated to this) Question is then what number that should show -- only the checksum counter in non-database-fields, or the total number across the cluster? -- Magnus Hagander Me: https://www.hagander.net/ <http://www.hagander.net/> Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
