Re: Support for 8-byte TOAST values, round two

Hey Michael, Bharath, all,

First, I've been following this on the earlier thread and this one and I
greatly appreciate the work you're doing Michael. I think it is valuable
and necessary. I've been out most of the Summer on vacation (I finally
took one!) and so haven't had the time until yesterday to dive into
this.

So, I applied and tested the v15 set and then the adjusted v16-0001 on
top of current master. All good, tests passing.

Beyond running the suite I poked at the feature directly. pg_resetwal -o
5000000000 followed by an insert into an oid8 table gives a chunk_id of
5000000000 and reads back correctly, while the oid table next to it
wraps to 705032705. pg_column_toast_chunk_id() reports oid8 and the full
value. I read the test body and found that your pg_upgrade tests do
assert the 2^32+100000 counter carries over, thanks for the
attention to detail.

I spent most of my time trying to put numbers on the objections already
raised, since several of them were estimates.

I think that Matthias is right about the index, and I've reproduced it
on real TOAST indexes, not a standalone proxy table. Same data on both
sides, 40000 chunks: 917504 bytes with oid against 1277952 with oid8, so
+39.3%. With denser 2-chunk values it's 1810432 against 2539520,
+40.3%. I see that Bharath's independent run at 4GB of TOAST landed on
+40.3% as well, so I'd treat the 40% as settled.

However, I'd add the other half of that ratio, because I think it
changes how much weight the number should carry. The TOAST heap was
byte-identical in both of my runs, and the index is only 1.1% to 1.6% of
TOAST storage there, 1.35% in Bharath's. So the extra bytes come to
under 0.7% of the total TOAST footprint.

Robert's last concern on the previous thread lingered and so I wanted to
check because it seems untested so far in practice. The thought was that
some tuples storable today would not be storable at all with the wider
pointer. I tried, but couldn't reproduce the concern in practice. I
created a table of N text columns, all set STORAGE EXTERNAL, 3000 bytes
per value:

cols oid oid8
360 OK OK
370 OK row is too big: size 8824, maximum size 8160
450 OK too big
460 too big too big

So the measured ceiling on out-of-line attributes per row goes from
about 453 to about 366, call it 19%. Between roughly 370 and 455 such
columns you get a row that stores fine as oid and cannot be stored at
all as oid8, and there is nothing the TOAST code can do about it. Wide
all-text tables like might exist, but how common are they? This is a
hard wall not a slowdown, so maybe a sentence in the docs next to the
reloption would help?

On the main heap, the cost is alignment dependent and sometimes
free. One toasted column came out byte-identical, 52.22 bytes per row
either way, because MAXALIGN swallows the extra 4 bytes. Two toasted
columns showed it: 68.40 against 76.60 bytes per row, +12%. Bharath
measured +15.4% on his shape. Worth knowing that the answer is "it
depends on the row layout" rather than a flat 4 bytes per value.

Which brings me to the v16-0001 doc change. I think this sentence is
stronger than the code supports:

The counter is 8 bytes wide and never wraps around during the life of
a cluster.

It can wrap, using the option that paragraph documents. pg_resetwal
happily accepts -o 18446744073709551615, and it rejects 2^64 with a
clean error, so the boundary itself is handled. But set the counter to
18446744073709550000, start up, insert 3000 toasted rows into an oid8
table, and the counter rolls over: chunk_ids run from 16384 to
18446744073709551615, 2778 of the 3000 rows land on low ids, and
next_oid after a checkpoint reads 24576. So it is possible to induce
that corner case, however unlikely in practice.

The good news is the wrap is handled the way the design says: it resumes
at FirstNormalObjectId rather than 0, all 6000 rows I ended up with
detoast correctly, all chunk_ids were distinct, and verify_heapam()
reports zero corruption rows on both the toast relation and the main
table. So this isn't a data integrity problem.

It does mean the invariant the oid8 path leans on is
administrator-breakable, though. Since there's no entry recheck for
oid8, monotonicity is what guarantees uniqueness, and after a wrap you
no longer have it. Your own example is the sharp version of this: reset
to a past value and the next insert fails hard on the toast index unique
constraint. I reproduced that too and it fails cleanly so the
pre-existing row's data was intact. It is a hard error and not silent
corruption.

Two smaller things. The duplicate key error a user hits in that state
names an internal toast index and gives no hint about the cause, so if
you want one place to spend a few lines it might be an errhint
there. And the reloption docs currently describe what toast_value_type
sets but none of the tradeoffs above, nor that dump and restore is the
only way to move an existing table. Bharath's point 4 established that
pg_dump does emit the changed reloption, so the migration path works, it
just isn't written down. Nit, there's also a stray double space in
"creating a toast relation".

On the bigger question of per-relation sequences, I agree with Robert
that bottlenecking this work behind that change is probably the wrong
trade. This fixes is a live defect that users hit today.

So: +1 from me on the direction and on this implementation. The v16-0001
counter work reads fine to me apart from that one doc sentence. I'll
keep going on 0006 and 0010, which are the two I've read least carefully
so far.

Reproducers for the row-size and wraparound cases are trivial to
reconstruct from the numbers above, but happy to post scripts if useful.

best.

-greg