[skunkwork] [adapter] Share allocators for all system / user id types #34296
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.
Traditionally, different types of objects may end up with what appears to be the same ID: so the index
u1may be created on replicau1of clusteru1. This is I think generally considered to be confusing.This change switches to assigning IDs that are unique across different types - so we will avoid creating a new cluster
u2if an objectu2already exists, and vice-versa. This is done by giving ourselves a way to assign a new id that will not conflict with multiple different types, and adopting that incrementally.This does not guarantee that there are no ID collisions across types, and in particular no legacy ID will change... but we do avoid conflicts in many cases, as the test changes show.
Motivation
First proposed in https://github.com/MaterializeInc/database-issues/issues/6336#issuecomment-2369825541.
I don't know if it has consensus as the right end state, but hopefully it is uncontroversial as an incremental step?
Tips for reviewer
One upshot of this is that IDs of things will change, including for some system things. I did try and ensure that the system cluster IDs wouldn't change, since I suspect we have those hardcoded in some alerts and other things, but as you can see from the test rewrites there are lots of IDs that will shift around by default. I think this is probably fine, but if you disagree, I am interested to hear about it!
I updated the regular CI tests, but there are probably some nightly failures that will crop up. I'll wait to resolve those until I think this is likely to be approved!
There are some other ID types that I haven't merged into the merged lists yet. I would prefer to save that for followup work, again assuming folks think this is a positive path.
An alternative to all this would be a catalog migration that collapses down all the different keys. This version is both more incremental and easier to reverse, but eventually something like that seems like it would make sense!