[#12576] improvement(core): optimize schema write fencing - #12577
Open
yuqi1129 wants to merge 10 commits into
Open
[#12576] improvement(core): optimize schema write fencing#12577yuqi1129 wants to merge 10 commits into
yuqi1129 wants to merge 10 commits into
Conversation
Advance the schema OCC version on every alter and guard alter and drop with a compare-and-set on the observed version, classifying a failed CAS as either a stale conflict or a missing entity. Make managed schema creation insert-only so a concurrent same-name create returns SchemaAlreadyExistsException instead of overwriting the winner, and take a shared lock on the parent catalog row so a schema cannot be created below a catalog that is being dropped. Serialize hierarchical ancestor materialization and schema drops through the catalog row so overlapping cascades share one lock order. Lock the parent schema row before writing a table, view, fileset, function, model, or topic, and check views and functions before a non-cascade schema drop. Accepted tradeoff: a hierarchical schema create that materializes implicit ancestors takes an exclusive lock on the catalog row, because two concurrent creates can both find the same ancestor missing and both insert it, and a shared lock does not prevent that under MySQL REPEATABLE READ.
… backends H2 is also the default embedded backend, not only a test backend. Spell out that falling back to an exclusive lock serializes schema creations under one catalog there and can surface as an H2 lock timeout.
…n code Review feedback: the concurrency-critical parts need comments so a reader can follow why the statements are ordered the way they are. - Say what the catalog row lock buys on a schema create, and why a nested name has to take it exclusively while a plain name does not. - Say why both drop paths delete the schema row before looking at its children, and why every drop takes catalog before schema. - Say what the shared schema lock in front of a table, view, fileset, function, model, or topic write is for, and that only a cross-schema rename needs it. - Say why the alter UPDATE compares only the version, what zero affected rows can mean, and why a partial cascade must roll back. - Say why managed schema creation is insert-only now. - Correct the schemaWriteFailure comment: sessions run at READ_COMMITTED, so the locking read is there to wait out an in-flight writer.
…ic on overwrite Carry the fix that apache#12455 already made for catalogs over to schemas, so both sides of the hierarchy follow the same rule. - Advance current_version on all four schema upsert paths (single and batch, on MySQL/H2 and PostgreSQL) instead of writing the initial version back, which would let a writer holding an older version still pass its own version check. - Name the table on the PostgreSQL assignments: a bare column on that side of ON CONFLICT is ambiguous there, which is how the previous CI run broke. - Add TestSchemaMetaPostgreSQLProvider to pin both rules without a database, so they are checked on every run and not only in the Docker-backed CI job. - Cover the race this PR is meant to close: a catalog cascade that holds the catalog row makes a concurrent schema create wait and then report the catalog as missing, leaving no orphan behind. Reading the cascade snapshot moved into a package-private method so the test can pause exactly at that point, the same seam MetalakeMetaService already offers. - Say that the H2 shared-lock fallback affects H2 backends, not just tests.
Code Coverage Report
Files
|
jerryshao
reviewed
Aug 25, 2026
| + TopicMetaMapper.TABLE_NAME | ||
| + " WHERE schema_id = #{schemaId} AND deleted_at = 0", | ||
| "LIMIT 1" | ||
| }) |
Contributor
There was a problem hiding this comment.
Shall we use our convention to define the SQL string in the provider, rather than add a notation here directly?
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.
What changes were proposed in this pull request?
This is a follow-up stacked on #12456. Until #12456 merges, GitHub also shows its commits in this draft; the #12576 follow-up itself is commit 10a17fe and changes 871 lines.
Why are the changes needed?
The previous implementation held the schema delete fence correctly, but every schema-scoped service had to assemble the lock and transaction manually. That made future omissions easy. Non-cascade deletion also materialized complete child objects and version information when it only needed a yes/no answer, extending the time spent under the schema delete lock.
Fix: #12576
Does this PR introduce any user-facing change?
No.
How was this patch tested?