On Tue, Apr 02, 2019 at 07:43:12AM +0200, Julien Rouhaud wrote: > On Tue, Apr 2, 2019 at 6:56 AM Michael Paquier <mich...@paquier.xyz> wrote: >> One thing which is not >> proposed on this patch, and I am fine with it as a first draft, is >> that we don't have any information about the broken block number and >> the file involved. My gut tells me that we'd want a separate view, >> like pg_stat_checksums_details with one tuple per (dboid, rel, fork, >> blck) to be complete. But that's just for future work. > > That could indeed be nice.
Actually, backpedaling on this one... pg_stat_checksums_details may be a bad idea as we could finish with one row per broken block. If a corruption is spreading quickly, pgstat would not be able to sustain that amount of objects. Having pg_stat_checksums would allow us to plugin more data easily based on the last failure state: - last relid of failure - last fork type of failure - last block number of failure. Not saying to do that now, but having that in pg_stat_database does not seem very natural to me. And on top of that we would have an extra row full of NULLs for shared objects in pg_stat_database if we adopt the unique view approach... I find that rather ugly. >> No need for two ifs here. What about just that? >> if (NULL) >> PG_RETURN_NULL(); >> else >> PG_RETURN_TIMESTAMPTZ(last_checksum_failure); > > I do agree, but this is done like this everywhere in pgstatfuncs.c, so > I think it's better to keep it as-is for consistency. Okay, this is not an issue for me. The patch looks fine to me as-is. Let's see what Magnus or others have to say about it. I can take care of this open item if necessary but that's not my commit so I'd rather not step on Magnus' toes. -- Michael
signature.asc
Description: PGP signature
