Re: tableam vs. TOAST

Hi,

On 2019-11-06 10:01:40 +0100, Peter Eisentraut wrote:
> On 2019-10-04 20:32, Robert Haas wrote:
> > Here's the last patch back, rebased over that renaming. Although I
> > think that Andres (and Tom) are probably right that there's room for
> > improvement here, I currently don't see a way around the issues I
> > wrote about inhttp://postgr.es/m/CA+Tgmoa0zFcaCpOJCsSpOLLGpzTVfSyvcVB-USS8YoKzMO51Yw@mail.gmail.com
> > -- so not quite sure where to go next. Hopefully Andres or someone
> > else will give me a quick whack with the cluebat if I'm missing
> > something obvious.
>
> This patch seems sound as far as the API restructuring goes.
>
> If I may summarize the remaining discussion: This patch adds a field
> toast_max_chunk_size to TableAmRoutine, to take the place of the hardcoded
> TOAST_MAX_CHUNK_SIZE. The heapam_methods implementation then sets this to
> TOAST_MAX_CHUNK_SIZE, thus preserving existing behavior. Other table AMs can
> set this to some other value that they find suitable. Currently,
> TOAST_MAX_CHUNK_SIZE is computed based on heap-specific values and
> assumptions, so it's likely that other AMs won't want to use that value.
> (Side note: Maybe rename TOAST_MAX_CHUNK_SIZE then.) The concern was raised
> that while TOAST_MAX_CHUNK_SIZE is stored in pg_control, values chosen by
> other table AMs won't be, and so they won't have any safe-guards against
> starting a server with incompatible disk layout. Then, various ways to
> detect or check the TOAST chunk size at run time were discussed, but none
> seemed satisfactory.

I think it's more than just that. It's also that I think presenting a
hardcoded value to the outside of / above an AM is architecturally
wrong. If anything this is an implementation detail of the AM, that the
AM ought to be concerned with internally, not something it should
present to the outside.

I also, and separately from that architectural concern, think that
hardcoding values like this in the control file is a bad practice, and
we shouldn't expand it. It basically makes it practically impossible to
ever change their default value.

> I think AMs are probably going to need a general mechanism to store
> pg_control-like data somewhere. There are going to be chunk sizes, block
> sizes, segment sizes, and so on. This one is just a particular case of
> that.

That's imo best done as a meta page within the table.

> This particular patch doesn't need to be held up by that, though. Providing
> that mechanism can be a separate subproject of pluggable storage.

Again seems like something that the AM ought to handle below it.

Greetings,

Andres Freund