lhjchn opened a new pull request, #11454:
URL: https://github.com/apache/gravitino/pull/11454
### What changes were proposed in this pull request?
Previously `DropCatalogStoredProcedure` only checked the Trino connector's
local cache (the `catalogConnectors` map) to decide whether a catalog exists.
When the local connector was null, it directly threw "not exists"
without checking the Gravitino server.
This PR changes the null branch to call
`metalake.dropCatalog(catalogName, true)` on the server:
1. If the server returns true, the catalog is dropped on the server
successfully.
2. If the server returns false, the catalog does not exist on the server
either. We then respect the `ignore_not_exist` flag: return silently if true,
throw `GRAVITINO_CATALOG_NOT_EXISTS` if false.
The branch where the local connector is found stays the same as before.
Also adds a unit test class `TestDropCatalogStoredProcedure` for the new
code path and updates the Javadoc.
### Why are the changes needed?
Fix: #11401
The local cache may not have the catalog even though it exists on the
Gravitino server. This can happen when:
1. The catalog is created with invalid properties and the connector fails to
load.
2. The cache entry is evicted.
3. Trino is restarted before the background scheduler reloads it.
In this state the catalog cannot be dropped (local check returns null,
`drop_catalog` throws "not exists") and cannot be recreated either (the server
still holds the metadata, `create_catalog` throws "already exists"). The
user has no way to recover without restarting Trino or calling the
server-side API directly.
By falling back to the server-side drop when the local cache misses, the
user can clean up the catalog with the same `drop_catalog` procedure.
### Does this PR introduce _any_ user-facing change?
No new API, configuration, or property key.
The behavior of `drop_catalog(catalog, ignore_not_exist)` changes only for
the bug case: calls that previously failed with "not exists" while the catalog
actually existed on the server now succeed. The other cases (catalog
truly does not exist anywhere; catalog exists in the local cache) behave the
same as before.
### How was this patch tested?
Added a new unit test class `TestDropCatalogStoredProcedure` with three
cases:
1. The local cache does not contain the catalog but the server does. The
procedure should call the server-side drop and complete without error.
2. The catalog is absent from both the local cache and the server, with
`ignoreNotExist=false`. The procedure should throw
`GRAVITINO_CATALOG_NOT_EXISTS`.
3. Same as case 2 but with `ignoreNotExist=true`. The procedure should
return silently and must not trigger a local cache reload.
Ran `./gradlew :trino-connector:trino-connector:test -PskipITs` and
`./gradlew :trino-connector:trino-connector:spotlessApply`, both pass.
<!--
1. Title: [#<issue>] <type>(<scope>): <subject>
Examples:
- "[#123] feat(operator): Support xxx"
- "[#233] fix: Check null before access result in xxx"
- "[MINOR] refactor: Fix typo in variable name"
- "[MINOR] docs: Fix typo in README"
- "[#255] test: Fix flaky test NameOfTheTest"
Reference: https://www.conventionalcommits.org/en/v1.0.0/
2. If the PR is unfinished, please mark this PR as draft.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
