Re: OOM in spgist insert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, May 13, 2021 at 6:20 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> OTOH, the 10-cycles-to-show-progress rule could be
>> argued to be an API break.

> Not being familiar with this code, I don't really understand why 10
> cycles to show progress wouldn't, like 640kB, be enough for anyone.

Yeah, after further thought I'm thinking that that ought to be plenty.
In released branches, that code will never execute at all unless the
datum-to-be-indexed is larger than ~8kB. If you are starting with,
say, a 30kB string, you'd better be removing a heck of a lot more than
one byte per tree level, or your search performance is going to be
abysmal. Maybe algorithmic oddities would sometimes result in
seemingly making no progress for one cycle, but I doubt there's need
for more slop than that. In this light, a 10-cycle grace period seems
if anything excessive.

> But as far as back-patching the code goals, I think the question is
> not so much whether this restriction could hypothetically break
> anything but whether it will actually break anything, which leads to
> the question of how many spgist opclasses we think exist outside of
> core.

That is also an interesting viewpoint. codesearch.debian.net knows
of no external SPGiST opclasses other than PostGIS'. They don't
seem to have indexed the two you found on github, though. None of
those four set longValuesOK to true, which means that the whole
discussion is moot for them. So it's entirely plausible that
spgtextproc.c is the only affected code anywhere.

Of course, that conclusion could lead to the position that there's
no point in back-patching anyway, since there's no reason to think
that spgtextproc.c is buggy.

regards, tom lane