Skip to content

[#12739] improvement(core): add OCC for user writes - #12744

Merged
yuqi1129 merged 2 commits into
apache:mainfrom
yuqi1129:feat/12739-occ-user
Sep 1, 2026
Merged

[#12739] improvement(core): add OCC for user writes#12744
yuqi1129 merged 2 commits into
apache:mainfrom
yuqi1129:feat/12739-occ-user

Conversation

@yuqi1129

@yuqi1129 yuqi1129 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add version-CAS optimistic concurrency control and atomic service operations for managed users.

  • Increment the user version on every update and match updates by user ID, expected version, and active-row state.
  • Make soft deletes version checked and classify a zero-row result as either a missing user or an OCC conflict.
  • Execute the user CAS before user-role and ownership mutations in one transaction.
  • Fence the parent metalake while creating a user and advance the version on overwrite upserts.
  • Add coverage for metadata-only updates, external-ID and ID updates, stale deletes, overwrite versioning, parent fencing, and relationship rollback.

Why are the changes needed?

Concurrent user mutations could otherwise overwrite each other or leave partially updated relationship state.

Fix: #12739

Does this PR introduce any user-facing change?

Concurrent managed-user writes now report the existing optimistic-lock conflict response (HTTP 409). No API or configuration keys are changed.

How was this patch tested?

  • ./gradlew :core:test --tests TestUserMetaService --tests TestAuthMappers --tests TestPOConverters -PskipITs -PskipDockerTests=true
  • ./gradlew :core:spotlessCheck :core:compileTestJava
  • Added a converter regression where currentVersion and lastVersion differ.
  • GitHub Backend Integration Test matrix: H2, MySQL, and PostgreSQL.

Copilot AI lite review requested due to automatic review settings August 31, 2026 08:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Every user write now carries the version it read. updateUserMeta,
updateUserMetaByExternalId and softDeleteUserMetaByUserId compare
current_version in the WHERE clause and report how many rows they
matched, so a writer working from a stale snapshot loses instead of
silently overwriting a newer row.

The predicate is the primary key plus the version rather than the whole
row. Comparing user_name, metalake_id, audit_info and last_version made
the statement look safe while keying on values that move together, and
it could not tell a lost update apart from a row renamed away.

updateUserPOWithVersion now advances the version instead of reusing the
last one, which is what makes the compare-and-set mean anything. The
overwrite insert advances it as well rather than writing the value from
the incoming PO, so a writer holding a pre-overwrite snapshot cannot
pass a later compare-and-set (an ABA conflict).

Creating a user takes a shared lock on the parent metalake row for the
rest of the transaction, so a create cannot slip in beside a drop of the
metalake it belongs to. The metalake's version is deliberately not
compared, matching CatalogMetaService: holding the row is what makes the
create safe, and an unrelated metalake edit committing in between would
otherwise reject the create for no reason.

A failed compare-and-set is classified by re-reading the row under a
lock. A row that is gone, renamed, or moved to another metalake is
reported as NoSuchEntityException; anything else is a concurrent
modification. What counts as "renamed away" depends on how the caller
addressed the user, which is what UserLookup records.

Two behavior changes worth calling out:

- updateUser no longer returns early when the role set is unchanged. The
  compare-and-set has to run so a stale caller is told, and a
  metadata-only change such as the audit info still has to be written.
- deleteUser now fails when the row moved under it. deleteUserById still
  returns false when the row is already gone, since a delete with
  nothing left to delete is a no-op rather than an error.
@yuqi1129
yuqi1129 force-pushed the feat/12739-occ-user branch from 664b711 to 1e1944b Compare August 31, 2026 09:43
@github-actions

Copy link
Copy Markdown

Code Coverage Report

Overall Project 68.61% +0.06% 🟢
Files changed 69.9% 🟢

Module Coverage
aliyun 19.74% 🔴
api 51.74% 🟢
authorization-common 85.96% 🟢
authorization-ranger 4.38% 🔴
aws 53.54% 🟢
azure 32.1% 🔴
catalog-common 19.1% 🔴
catalog-fileset 80.3% 🟢
catalog-glue 69.24% 🟢
catalog-hive 82.96% 🟢
catalog-jdbc-common 45.69% 🟢
catalog-jdbc-doris 82.44% +2.03% 🟢
catalog-jdbc-mysql 79.33% 🟢
catalog-jdbc-postgresql 83.39% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 76.99% 🟢
catalog-lakehouse-generic 60.55% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.93% 🟢
catalog-lakehouse-paimon 84.26% 🟢
catalog-model 77.99% 🟢
cli 44.48% 🟢
client-java 77.44% +0.08% 🟢
common 55.5% 🟢
core 83.66% -0.2% 🟢
filesystem-hadoop3 76.45% 🟢
flink 0.0% 🔴
flink-common 50.29% -9.3% 🟢
flink-runtime 0.0% 🔴
gcp 32.2% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 17.84% 🔴
hive-metastore-common 53.4% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 64.75% 🟢
iceberg-rest-server 75.96% 🟢
idp-basic 85.98% 🟢
integration-test-common 0.0% 🔴
jobs 62.92% 🟢
lance-common 32.63% 🔴
lance-rest-server 65.46% 🟢
lineage 53.02% 🟢
optimizer 83.17% 🟢
optimizer-api 21.95% 🔴
server 88.11% 🟢
server-common 80.5% 🟢
spark 28.57% 🔴
spark-common 48.92% 🟢
tencent 81.78% 🟢
trino-connector 47.44% +2.74% 🟢
Files
Module File Coverage
catalog-jdbc-doris DorisTableOperations.java 84.43% 🟢
client-java GravitinoClientBase.java 80.9% 🟢
core ModelVersionAliasRelBaseSQLProvider.java 100.0% 🟢
ModelVersionMetaBaseSQLProvider.java 100.0% 🟢
UserMetaBaseSQLProvider.java 100.0% 🟢
ModelVersionAliasRelH2SQLProvider.java 100.0% 🟢
ModelVersionMetaH2Provider.java 100.0% 🟢
ModelMetaPostgreSQLProvider.java 100.0% 🟢
ModelVersionAliasRelPostgreSQLProvider.java 100.0% 🟢
ModelVersionMetaPostgreSQLProvider.java 100.0% 🟢
UserMetaPostgreSQLProvider.java 100.0% 🟢
ModelPO.java 97.83% 🟢
UserMetaSQLProviderFactory.java 96.67% 🟢
UserMetaService.java 96.23% 🟢
ModelVersionMetaSQLProviderFactory.java 95.65% 🟢
ModelVersionAliasSQLProviderFactory.java 95.45% 🟢
ModelVersionMetaService.java 93.64% 🟢
POConverters.java 88.31% 🟢
TopicMetaPostgreSQLProvider.java 87.5% 🟢
ModelMetaSQLProviderFactory.java 83.33% 🟢
TopicMetaSQLProviderFactory.java 79.31% 🟢
ModelMetaService.java 76.72% 🟢
ModelMetaBaseSQLProvider.java 75.0% 🟢
TopicMetaService.java 72.48% 🟢
AuxiliaryServiceManager.java 69.47% 🟢
TopicMetaBaseSQLProvider.java 68.42% 🟢
GravitinoEnv.java 29.82% 🔴
ModelMetaMapper.java 0.0% 🔴
ModelVersionAliasRelMapper.java 0.0% 🔴
ModelVersionMetaMapper.java 0.0% 🔴
TopicMetaMapper.java 0.0% 🔴
UserMetaMapper.java 0.0% 🔴
flink-common BaseCatalog.java 41.49% 🔴
GravitinoHiveCatalog.java 6.67% 🔴
trino-connector IcebergCatalogPropertyConverter.java 86.67% 🟢
GravitinoConfig.java 85.71% 🟢
GravitinoConnectorFactory.java 67.74% 🟢
IcebergConnectorAdapter.java 66.67% 🟢
DefaultCatalogConnectorFactory.java 56.52% 🔴
CatalogRegister.java 50.25% 🔴
GravitinoConnectorPluginManager.java 41.22% 🔴
CatalogConnectorManager.java 34.24% 🔴

@yuqi1129 yuqi1129 self-assigned this Sep 1, 2026
@yuqi1129
yuqi1129 merged commit ff3a9e7 into apache:main Sep 1, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Subtask] Implement version-CAS OCC for user

3 participants