Re: [PATCH] Infinite loop while acquiring new TOAST Oid

Hi,

Andres Freund <andres(at)anarazel(dot)de> writes:
>Was the issue that you exceeded 4 billion toasted datums, or that
assignment
>took a long time? How many toast datums did you actually have? Was this
due to
>very wide rows leading to even small datums getting toasted?

Yep, we had 4 billion toasted datums. I remind that currently relation has
single
TOAST table for all toastable attributes, so it is not so difficult to get
to 4 billion
of toasted values.

>I think if we switch to int8 keys and widen the global OID counter to 8
>bytes (using just the low 4 bytes for other purposes), we'll have a
>perfectly fine solution. There is still plenty of work to be done under
>that plan, because of the need to maintain backward compatibility for
>existing TOAST tables --- and maybe people would want an option to keep on
>using them, for non-enormous tables? If we add additional work on top of
>that, it'll just mean that it will take longer to have any solution.

I agree, but:
1) Global OID counter is used not only for TOAST, so there would be a lot of
places where the short counter (low part of 64 OID, if we go with that) is
used;
2) Upgrading to 64-bit id would require re-toasting old TOAST tables. Or
there
is some way to distinguish old tables from new ones?

But I don't see any reason to keep an old short ID as an option.

...
>Yeah, that is completely horrid. It does not remove the existing failure
>mode, just changes it to have worse consequences.

On Tue, Nov 29, 2022 at 12:04 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Andres Freund <andres(at)anarazel(dot)de> writes:
> > I think the first step to improve the situation is to not use a global
> oid
> > counter for toasted values. One way to do that would be to use the
> sequence
> > code to do oid assignment, but we likely can find a more efficient
> > representation.
>
> I don't particularly buy that, because the only real fix is this:
>
> > Eventually we should do the obvious thing and make toast ids 64bit wide
>
> and if we do that there'll be no need to worry about multiple counters.
>
> > - to
> > combat the space usage we likely should switch to representing the ids as
> > variable width integers or such, otherwise the space increase would
> likely be
> > prohibitive.
>
> And I don't buy that either. An extra 4 bytes with a 2K payload is not
> "prohibitive", it's more like "down in the noise".
>
> I think if we switch to int8 keys and widen the global OID counter to 8
> bytes (using just the low 4 bytes for other purposes), we'll have a
> perfectly fine solution. There is still plenty of work to be done under
> that plan, because of the need to maintain backward compatibility for
> existing TOAST tables --- and maybe people would want an option to keep on
> using them, for non-enormous tables? If we add additional work on top of
> that, it'll just mean that it will take longer to have any solution.
>
> >> Quick fix for this problem is limiting GetNewOidWithIndex loops to some
> >> reasonable amount defined by related macro and returning error if there
> is
> >> still no available Oid. Please check attached patch, any feedback is
> >> appreciated.
>
> > This feels like the wrong spot to tackle the issue.
>
> Yeah, that is completely horrid. It does not remove the existing failure
> mode, just changes it to have worse consequences.
>
> regards, tom lane
>

--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/