On Tue, Feb 25, 2025 at 12:36:40PM -0500, Robert Treat wrote:
> On Tue, Feb 25, 2025 at 11:32 AM Nathan Bossart
> <nathandbossart@gmail.com> wrote:
>> I'm currently leaning towards the "remove all caps" idea. But I'm not sure
>> I totally understand the need for a WARNING. What do we expect users to do
>> with that information? If it's intended to alert them of possible
>> corruption, then IMHO a WARNING may be too gentle. I guess we could warn
>> and suggest a way to fix the value with the new statistics manipulation
>> functions if it's not that big of a deal, but at that point we might as
>> well just cap it on our own again. If we don't really expect users to have
>> to do anything about it, then isn't it just adding unnecessary log noise?
>
> If the end user is manipulating numbers to test some theory, I think
> it's valuable feedback that they have probably bent the system too
> far, because we are now seeing numbers that don't make sense.
If we do that in other cases, then that seems reasonable. But it feels
weird to me to carve out just this one inaccuracy. For example, do we warn
if someone sets reltuples too high for relpages, or if they set relpages
too low for reltuples? I'm not seeing why the relallfrozen <=
relallvisible <= relpages case is especially important to uphold. If we
were consistent about enforcing these kinds of invariants everywhere, then
I think I would be more on board with capping the values and/or ERROR-ing
when folks tried to set bogus ones. But if the only outcome of bogus
values is that you might get weird plans or autovacuum might prioritize the
table differently, then I'm not sure I see the point. You can accomplish
both of those things with totally valid values already.
> If they
> aren't mucking with the system, then it's valuable feedback that they
> may have an underlying system problem that could be about to get
> worse.
Maybe a WARNING is as much as we can realistically do in this case, but I
think it could be easily missed in the server logs. I dunno, it just feels
wrong to me to deal with potential corruption by gently notifying the user
and proceeding normally. I guess that's what we do already elsewhere,
though (e.g., for relfrozenxid/relminmxid in vac_update_relstats()).
--
nathan
