Hi, On Wed, Sep 2, 2026 at 9:58 PM Michael Paquier <[email protected]> wrote: > > On Wed, Sep 02, 2026 at 01:27:00PM -0700, Bharath Rupireddy wrote: > > I first tried to reproduce the query latency increase problem with > > less time and disk space. Here are the results. I ensured each insert > > steps over K ids using pg_resetwal, which equals the number of > > iterations spent in GetNewOidWithIndex(). There's a clear benefit as > > the 8-byte chunk_id avoids the retries while getting the new OID, and > > since a 64-bit OID is almost never exhausted, this works. Attached the > > test script that I used for reference. > > > > K HEAD_INSERT patched_INSERT > > 1,000,000 1.25 s 1.0 ms > > 10,000,000 12.4 s 1.0 ms > > 50,000,000 1 min 1.0 ms > > 100,000,000 2 min 1.0 ms > > 200,000,000 9 min 0.9 ms > > This measures how much time we take to grab a free OID value, hoping > that there is a hole.
Yes, not all the chunk_ids are occupied in this testing, so there are holes. I wanted to keep the testing time to a minimum and show how long it usually takes for insert/update queries to fetch a chunk_id when it's closer to the 4 billion limit. I attached the test script used for this testing (courtesy: Claude Code). Do you have any other test cases in mind? > > Comments on 0001: > > > > 1/ > > +table_relation_fetch_toast_slice(Relation toastrel, Oid8 valueid, > > While changing Oid to Oid8 is fine for internal functions, is it okay > > to change it for the table_relation_fetch_toast_slice table AM? Is > > there any chance that an external table AM forgets to update? I'm not > > arguing against this change, just checking if there's any way out > > here. Of course, the compiler will generate an incompatible function > > parameter type warning, and since the change is going in a major > > release, that seems fine. One idea could be to have a TOAST version > > field elsewhere to detect such changes, but that seems overkill. So, > > having it like 0001 seems fine to me unless others have any thoughts. > > PS: one option is to send the 8-byte OID via a caller-allocated > > varlena result pointer, but this seems a bit ugly. > > Yes, assuming that a table AM uses its own fetch_toast_slice(), it > will need to update probably for its own ScanKeyInit() if an external > TOAST table is in use for the OID8 case, assuming that they need to do > so based on the reloption for the toast value type. I am not really > convinced that we need to be fancy here, telling that after looking at > out-of-code projets that include their own callbacks: > https://github.com/eatonphil/pgtam (no support) > https://github.com/rohankumardubey/pg_mooncake > https://github.com/neurdb/neurdb (no support) > https://github.com/duckdb/pg_duckdb (totally different callback here) > https://github.com/timescale/timescaledb > https://github.com/jeffreydwalter/pg_tde/ (needs refresh) > https://github.com/ewhauser/unitpg/ (just reuses the heap call) > > There are also a few more, but I am seeing nothing beyond an extra > ScanKeyInit() update required, in a sea of non-supported cases. Thanks for providing these details. It may help table AM implementers to have a note in the commit message, the docs, or this thread explaining how they need to adapt. > > Widening the per-toast-chunk hash key to an 8-byte OID still handles > > 4-byte chunk_ids correctly during logical decoding, since the smaller > > values zero-extend. The tradeoff is that with an 8-byte key the hash > > no longer uses the uint32_hash fast path and falls back to tag_hash, > > so even existing 4-byte chunk_id tables lose that optimization during > > logical decoding. I haven't measured the effect, but it seems worth > > checking. > > Yeah, point taken. I'm putting a note down on this one. Also, I think there's an issue in ReorderBufferToastAppendChunk() with 0001. The chunk_id is still fetched as a 4-byte value, so past 4 billion chunk_ids the truncated key stored in the TOAST hash won't match the 64-bit value used for the lookup later in ReorderBufferToastReplace(). > > Comments on 0008: > > > > 1/ I think adding reloption seems okay to me, so the 0008 patch looks > > good. Just one suggestion: having a GUC to set it once for all new > > tables seems a good idea as it avoids application changes, but > > starting with reloption is good enough. > > The round one of the patch set used a GUC. Andres has argued in favor > of a reloption. That works for me. A quick question, though I haven't tested this. Does ALTER TABLE error out if the reloption is changed on an existing table? > > I will take a look at 0006, 0007, 0009, 0010, 0011 in the coming weeks. > > Cool, thanks. > > > One more thought: existing tables won't get 8-byte chunk_ids as part > > of pg_upgrade. pg_dump and pg_restore could be used after the upgrade > > for existing customers to get their tables onto this format. At some > > point, providing concurrent repack-like support (or reusing the > > underlying machinery) to do this online would be nice to have. > > The infrastructure that could be used to switch the toast value type > and its rewrite is out of scope, the patch being complicated enough.. > We could always think about that later, giving the choice in core is > much more important to me as a first step, because without the > OID8/OID choice, there is no discussion about the rewrite part. That's fine by me. That said, it would be good to document at least one clear way for existing users to move their tables to 8-byte TOAST chunk_ids, presumably pg_dump and pg_restore after setting the reloption. I'm also wondering if pg_dump needs an option to generate table schemas with the new reloption set, so that users don't have to edit the dump file. Editing may not always be possible (for example, with the custom and directory formats). This can be a follow-up patch, but having at least one supported way to migrate existing tables seems important to me. On Thu, Sep 3, 2026 at 9:28 PM Michael Paquier <[email protected]> wrote: > > After an extra set of checks, I am dropping the catcache thing for > OID8OID, for the simple reason that there is no need for it anymore. > I am pretty sure that I did some OID8 catalog lookup at some point of > this patch set, but nothing shows up now. Perhaps that was in the > round 1 of the discussion. Agreed, dropping it seems right at a quick glance. The chunk_id comes from the heap row's TOAST pointer and is used as a scan key against the TOAST index, so nothing looks up an OID8 value through the catalog cache. > I'll post a rebase of the remaining pieces once the rename thread is > completely settled, hopefully around the beginning of next week. Thanks. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
nocfbot_toast_oid.sh
Description: Bourne shell script
