[adapter] Batch-allocate user IDs to reduce persist writes during DDL#35310
Open
mtabebe wants to merge 1 commit intoMaterializeInc:mainfrom
Open
[adapter] Batch-allocate user IDs to reduce persist writes during DDL#35310mtabebe wants to merge 1 commit intoMaterializeInc:mainfrom
mtabebe wants to merge 1 commit intoMaterializeInc:mainfrom
Conversation
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
a0cba20 to
563d8af
Compare
mtabebe
commented
Mar 3, 2026
| let (connection_id, connection_gid) = match self.allocate_user_id().await { | ||
| Ok(item_id) => item_id, | ||
| Err(err) => return ctx.retire(Err(err.into())), | ||
| Err(err) => return ctx.retire(Err(err)), |
Contributor
Author
There was a problem hiding this comment.
I got hit with a lint issue about this, so tidying it up
c3dab3b to
092af75
Compare
Problem: Every DDL statement (CREATE TABLE, CREATE VIEW, etc.) performs a persist write + timestamp oracle call to allocate a single user ID. This represents one extra round trip to the timestamp oracle, in addition to the necessary catalog operation for DDL. Solution: - Add `IdPool` struct with `allocate()`, `allocate_many()`, and `refill()` methods to pre-allocate user IDs in a contiguous range - Add `USER_ID_POOL_BATCH_SIZE` dyncfg (default 512) to control batch size - Add `user_id_pool` field to `Coordinator` initialized as empty - Add Coordinator wrapper methods `allocate_user_id()` and `allocate_user_ids(count)` that draw from the pool and auto-refill from catalog when exhausted - Replace all call sites that used the two-line pattern of `get_catalog_write_ts()` + `catalog().allocate_user_id(ts)` with the new pooled methods - This eliminates the timestamp oracle call, and catalog operation Testing: - New `IdPool` unit tests covering empty pool, single/batch allocation, refill, mixed allocation, and invalid range panic - New SLT regression test (`test/sqllogictest/id_pool.slt`) exercising tables, views, materialized views, indexes, types, secrets, and drop/recreate cycles - New MZCompose restart test to ensure IDs remain unique across the restart
092af75 to
d3ee143
Compare
Contributor
|
Nightly triggered: https://buildkite.com/materialize/nightly/builds/15452 |
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.
Problem:
Every DDL statement (CREATE TABLE, CREATE VIEW, etc.) performs a persist write + timestamp oracle call to allocate a single user ID. This represents one extra round trip to the timestamp oracle, in addition to the necessary catalog operation for DDL.
Solution:
IdPoolstruct withallocate(),allocate_many(), andrefill()methods to pre-allocate user IDs in a contiguous rangeUSER_ID_POOL_BATCH_SIZEdyncfg (default 512) to control batch sizeuser_id_poolfield toCoordinatorinitialized as emptyallocate_user_id()andallocate_user_ids(count)that draw from the pool and auto-refill from catalog when exhaustedget_catalog_write_ts()+catalog().allocate_user_id(ts)with the new pooled methodsTesting:
IdPoolunit tests covering empty pool, single/batch allocation, refill, mixed allocation, and invalid range panictest/sqllogictest/id_pool.slt) exercising tables, views, materialized views, indexes, types, secrets, and drop/recreate cycles