On Fri, Mar 17, 2023 at 4:11 PM Michael Paquier <michael@paquier.xyz> wrote:
> FWIW, I am not sure that it is a good idea and that we'd better not
> encourage too much the use of block_info() across a large range of
> WAL, which is what this function will make users eager to do in this
> case as it is possible to apply directly more filters to it.
I'm sure that they will do that much more than they would have
otherwise. Since we'll have made pg_get_wal_block_info() so much more
useful than pg_get_wal_records_info() for many important use cases.
Why is that a bad thing? Are you concerned about the overhead of
pulling in FPIs when pg_get_wal_block_info() is run, if Bharath's
patch is committed? That could be a problem, I suppose -- but it would
be good to get more data on that. Do you think that this will be much
of an issue, Bharath?
I have pushed pg_walinspect to its limits myself (which is how I found
that memory leak). Performance matters a great deal when you're doing
an analysis of how blocks change over time, on a system that has
written a realistically large amount of WAL over minutes or even
hours. Why shouldn't that be a priority for pg_walinspect? My concerns
have little to do with aesthetics, and everything to do with making
those kinds of queries feasible.
If the FPI thing is a problem then it seems to me that it should be
addressed directly. For example, perhaps it would make sense to add a
way to not incur the overhead of decompressing FPIs uselessly in cases
where they're of no interest to us (likely the majority of cases once
the patch is committed). It also might well make sense to rename
pg_get_wal_block_info() to something more general, to reflect its more
general purpose once it is expanded by Bharath's patch. As I said, it
will become a lot closer to pg_get_wal_records_info(). We should be
clear on that.
--
Peter Geoghegan
