Handle custom types with differing OIDs across shards - #1280
Open
sgrif wants to merge 6 commits into
Open
Conversation
Contributor
Author
|
This is now a working implementation. It needs some cleanup and backfill of unit tests, but should be able to make it into tomorrow's release |
sgrif
commented
Jul 29, 2026
| /// A combination of `SetOnce` and `OnceCell`. | ||
| /// It allows only a single writer to run, with no contention on future reads, | ||
| /// while also allowing callers to wait on the value. | ||
| pub(crate) struct SetOnceCell<T> { |
Contributor
Author
There was a problem hiding this comment.
Prior art: tokio-rs/tokio#4788 https://doc.rust-lang.org/stable/std/sync/struct.OnceLock.html
(Likely going to try to upstream this to tokio since it was previously rejected as a new feature on OnceCell, but it was mentioned it may be accepted as a new type)
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
levkk
reviewed
Jul 30, 2026
levkk
reviewed
Jul 30, 2026
At the wire protocol level, type information is entirely handled by OID, not type name. An OID is a unique identifier that PG assigns for virtually every schema related entity. They are extremely dependent on the order that plugins are loaded, DDL is run, etc. Because of this it's quite likely that a user or extension defined type will not have the same OID across multiple shards. To solve this, we need to do three things: - Ensure clients receive a single canonical answer when they query for type information - Rewrite any messages being sent to the server to ensure they have the correct OIDs for a given type for that sever - Rewrite any messages being sent to the client to ensure they have the canonical OIDs for a given type This commit's solution to ensuring the client gets canoincal type information is straightforward, but not ideal. We look for any select statements that could be loading type information (referencing the relevant tables in `pg_catalog`, or doing certain casts), and route them to shard 0. This means that we have a single point of failure for schema information. Ideally we would be responding to these queries ourselves rather than sending them to the server, but the breadth of different ways different client libraries will load this type information would mean we essentially have to implement our own shitty SQL server. It's possible, and probably necessary in the long run, but outside of the scope of this specific fix. It's not clear that there'd be any reason one shard would be better than another for the canonical information, but if there is one, we could probably surface this as a configuration option. But since there's no clear reason for it, we can just do the reasonable default. For rewriting messages, we need to keep track of the mappings for each shard on our end. This was actually somewhat tricky to make work, as schema loading happens on `Shard`, but the message rewriting needs to happen all the way down in `Server`, and these are not types that share an API bounary. To rectify this, we use a new primitive, which is the async equivalent of `std::sync::OnceLock`. It ensures that only one writer will ever attempt to write to it, while also allowing waiters to wait for a value to be set without attempting to initialize themselves. With this primitive, all we need to do is stick it in an `Arc` to have it shared in as many disparate places as we need. We use the `SchemaCache` that was previously introduced to ensure we only have one copy of this mapping per database, regardless of how many user/clusters we have, but its use is relatively minimal. Since this sits in a very hot path, I've done as much as I could to avoid excess allocations or loops if they're not strictly necessary. I'm assuming that the majority of users won't have custom types that vary between shards, so this change should ideally be free for those users, short of the query to determine that there are no mappings at startup. Tests were a little bit trickier. Resolving the OIDs happens at cluster launch, and the code that needs to do the rewriting will never run without the cluster launching and loading first. But many of our tests were constructing clusters without launching them. I worked around this by making `Oids::default` return an instance that is already resolved to an empty set of mappings. Since the normal construction requires canonical type information to be passed in, we don't need to worry about production code accidentally calling this incorrectly.
sgrif
marked this pull request as ready for review
July 30, 2026 20:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Handle custom types with differing OIDs across shards
At the wire protocol level, type information is entirely handled by OID, not type name. An OID is a unique identifier that PG assigns for virtually every schema related entity. They are extremely dependent on the order that plugins are loaded, DDL is run, etc. Because of this it's quite likely that a user or extension defined type will not have the same OID across multiple shards.
To solve this, we need to do three things:
type information
correct OIDs for a given type for that sever
canonical OIDs for a given type
This commit's solution to ensuring the client gets canoincal type information is straightforward, but not ideal. We look for any select statements that could be loading type information (referencing the relevant tables in
pg_catalog, or doing certain casts), and route them to shard 0.This means that we have a single point of failure for schema information. Ideally we would be responding to these queries ourselves rather than sending them to the server, but the breadth of different ways different client libraries will load this type information would mean we essentially have to implement our own shitty SQL server. It's possible, and probably necessary in the long run, but outside of the scope of this specific fix.
It's not clear that there'd be any reason one shard would be better than another for the canonical information, but if there is one, we could probably surface this as a configuration option. But since there's no clear reason for it, we can just do the reasonable default.
For rewriting messages, we need to keep track of the mappings for each shard on our end. This was actually somewhat tricky to make work, as schema loading happens on
Shard, but the message rewriting needs to happen all the way down inServer, and these are not types that share an API bounary.To rectify this, we use a new primitive, which is the async equivalent of
std::sync::OnceLock. It ensures that only one writer will ever attempt to write to it, while also allowing waiters to wait for a value to be set without attempting to initialize themselves.With this primitive, all we need to do is stick it in an
Arcto have it shared in as many disparate places as we need. We use theSchemaCachethat was previously introduced to ensure we only have one copy of this mapping per database, regardless of how many user/clusters we have, but its use is relatively minimal.Since this sits in a very hot path, I've done as much as I could to avoid excess allocations or loops if they're not strictly necessary. I'm assuming that the majority of users won't have custom types that vary between shards, so this change should ideally be free for those users, short of the query to determine that there are no mappings at startup.
Tests were a little bit trickier. Resolving the OIDs happens at cluster launch, and the code that needs to do the rewriting will never run without the cluster launching and loading first. But many of our tests were constructing clusters without launching them.
I worked around this by making
Oids::defaultreturn an instance that is already resolved to an empty set of mappings. Since the normal construction requires canonical type information to be passed in, we don't need to worry about production code accidentally calling this incorrectly.