lhjchn commented on code in PR #11417:
URL: https://github.com/apache/gravitino/pull/11417#discussion_r3356809144
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/DropCatalogStoredProcedure.java:
##########
@@ -79,25 +79,48 @@ public Procedure createStoredProcedure() throws
NoSuchMethodException, IllegalAc
/**
* Drops the specified catalog.
*
+ * <p>If the catalog is not present in the local connector cache, this
method falls back to a
+ * server-side drop against the Gravitino server. The local cache may
legitimately be missing the
+ * entry (e.g. the connector failed to load at startup, or the cache was
evicted), so consulting
+ * the server avoids leaving the catalog in a zombie state where it can be
neither dropped nor
+ * re-created. Only when the server also reports that the catalog does not
exist is {@code
+ * ignoreNotExist} consulted to decide between silent return and an error.
+ *
* @param catalogName the name of the catalog to drop
- * @param ignoreNotExist whether to ignore if the catalog does not exist
(only checked when the
- * catalog cannot be found initially)
+ * @param ignoreNotExist whether to ignore if the catalog does not exist on
both the local cache
+ * and the Gravitino server
* @throws TrinoException if the catalog does not exist and ignoreNotExist
is false
*/
public void dropCatalog(String catalogName, boolean ignoreNotExist) {
try {
CatalogConnectorContext catalogConnector =
catalogConnectorManager.getCatalogConnector(
catalogConnectorManager.getTrinoCatalogName(metalake,
catalogName));
+
+ // The local in-memory cache may not have an entry even if the catalog
exists on
+ // the Gravitino server (e.g. the connector failed to load at startup,
or the cache was
+ // evicted). In that case the catalog would otherwise be stuck in a
zombie state:
+ // it cannot be dropped here because the local check would say "not
exists", and it
+ // cannot be re-created either because the server still holds the
metadata.
+ // Fall back to the server-side drop so the server is the source of
truth.
if (catalogConnector == null) {
Review Comment:
1. You're right, I'll remove the "cache was evicted" wording.
2. The user sees the catalog via CLI/REST/UI, just not from Trino.
create_catalog with bad props succeeds on server but fails to load locally, so
drop_catalog says "not exists" while create_catalog says "already exists".
--
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]
