[#11262] fix: Clean up orphaned schema entities after table or view drop#11362
Open
roryqi wants to merge 4 commits into
Open
[#11262] fix: Clean up orphaned schema entities after table or view drop#11362roryqi wants to merge 4 commits into
roryqi wants to merge 4 commits into
Conversation
60e98e7 to
f278cd9
Compare
Code Coverage Report
Files
|
…splitSchemaName Replace raw split(Pattern.quote(separator)) calls in IcebergNamespaceHookDispatcher with the shared HierarchicalSchemaUtil.splitSchemaName util and drop the now-unused java.util.regex.Pattern import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…best-effort and de-duplicate - Make SchemaEntityCleaner.deleteOrphanedSchemaEntities best-effort: it now logs and swallows store/probe failures instead of throwing, so a transient error during the secondary orphan cleanup no longer fails an already-successful table/view/schema drop. - Extract the duplicated best-effort cleanup in IcebergTableHookDispatcher and IcebergViewHookDispatcher into a shared IcebergOrphanSchemaCleanup helper. - Use HierarchicalSchemaUtil.splitSchemaName instead of raw split(Pattern.quote(separator)) in the Iceberg table/view drop cleanup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts: # iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
Contributor
Author
|
@mchades Could u help me take a look? |
mchades
reviewed
Jun 3, 2026
Comment on lines
+440
to
+444
| schemaIdent -> | ||
| doWithCatalog( | ||
| catalogIdent, | ||
| c -> c.doWithSchemaOps(s -> s.schemaExists(schemaIdent)), | ||
| RuntimeException.class)); |
Contributor
There was a problem hiding this comment.
Can we retrieve the results in a single call to listSchemas to avoid multiple invocations of the underlying catalog?
Contributor
Author
There was a problem hiding this comment.
Iceberg doesn't support the similar operation now. But this improvement is discussing. Maybe we can optimize this point in the feature.
Contributor
Author
There was a problem hiding this comment.
You can see the discussion https://lists.apache.org/thread/o6ojgqyzrpwxp1wb0onl7wxmrxrfgx46
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 PR cleans up Gravitino schema entities that become orphaned when the underlying catalog automatically removes empty parent namespaces after dropping a table, view, or namespace.
The patch:
Why are the changes needed?
Some catalogs, including Iceberg JdbcCatalog, may implicitly remove empty namespaces and their empty parent namespaces after the last table/view/sub-namespace is dropped. Gravitino could still keep the corresponding schema entities in its store, leaving stale metadata.
Fix: #11262
Does this PR introduce any user-facing change?
No new API, configuration, or user-facing behavior is introduced. This fixes stale internal metadata cleanup after drop operations.
How was this patch tested?
git diff --check