Re: Checksum errors in pg_stat_database

On Sat, Apr 13, 2019 at 8:46 PM Robert Treat <rob(at)xzilla(dot)net> wrote:

>
> On Fri, Apr 12, 2019 at 8:18 AM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> > On Sun, Apr 7, 2019 at 6:28 PM Julien Rouhaud <rjuju123(at)gmail(dot)com>
> wrote:
> >> Thanks for looking it it!
> >> On Sun, Apr 7, 2019 at 4:36 PM Magnus Hagander <magnus(at)hagander(dot)net>
> wrote:
> >> >
> >> > I'm not sure I like the idea of using "<shared_objects>" as the
> database name. It's not very likely that somebody would be using that as a
> name for their database, but i's not impossible. But it also just looks
> strrange. Wouldn't NULL be a more appropriate choice?
> >> >
> >> > Likewise, shouldn't we return NULL as the number of backends for the
> shared counters, rather than 0?
> >> I wanted to make things more POLA-compliant, but maybe it was a bad
> >> idea. I changed it for NULL here and for numbackends.
> >>
>
> ISTM the argument here is go with zero since you have zero connections
> vs go with null since you can't actually connect, so it doesn't make
> sense. (There is a third argument about making it -1 since you can't
> connect, but that breaks sum(numbackends) so it's easily dismissed.) I
> think I would have gone for 0 personally, but what ended up surprising
> me was that a bunch of other stuff like xact_commit show zero when
> AFAICT the above reasoning would apply the same to those columns.
> (unless there is a way to commit a transaction in the global objects
> that I don't know about).
>

That's a good point. I mean, you can commit a transaction that involves
changes of global objects, but it counts in the database that you were
conneced to.

We should probably at least make it consistent and make it NULL in all or 0
in all.

I'm -1 for using -1 (!), for the very reason that you mention. But either
changing the numbackends to 0, or the others to NULL would work for
consistency. I'm leaning towards the 0 as well.

>> > Micro-nit:
> >> > + <entry>Time at which the last data page checksum failures was
> detected in
> >> > s/failures/failure/
> >>
> >> Oops.
> >>
> >> v5 attached.
> >
>
> What originally got me looking at this was the idea of returning -1
> (or maybe null) for checksum failures for cases when checksums are not
> enabled. This seems a little more complicated to set up, but seems
> like it might ward off people thinking they are safe due to no
> checksum error reports when they actually aren't.
>

NULL seems like the reasonable thing to return there. I'm not sure what
you're referring to with a little more complicated to set up, thought? Do
you mean somehow for the end user?

Code-wise it seems it should be simple -- just do an "if checksums disabled
then return null" in the two functions.

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