This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9223348fc8 [#13809] fix(core): Clarify catalog drop errors for
unreachable backends (#13810)
9223348fc8 is described below
commit 9223348fc83a781a409333af53b5327817cba2a6
Author: mchades <[email protected]>
AuthorDate: Tue Oct 13 14:19:59 2026 +0800
[#13809] fix(core): Clarify catalog drop errors for unreachable backends
(#13810)
### What changes were proposed in this pull request?
Add schema-check context and a `force=true` hint when dropping a catalog
with an unreachable backend. Preserve the original cause and the full
error message in the REST response.
### Why are the changes needed?
The current error does not explain that backend access is required for
the pre-drop schema check or how to skip that check.
Fix: #13809
### Does this PR introduce _any_ user-facing change?
Clearer errors for unreachable backends. Wrapped
`ConnectionFailedException`s follow the existing HTTP 502 mapping.
Deletion semantics remain unchanged.
### How was this patch tested?
62 focused tests passed across TestCatalogManager,
TestCatalogOperations, and TestExceptionHandlers. Core and server
Spotless checks passed.
---
.../apache/gravitino/catalog/CatalogManager.java | 28 ++++++-
.../gravitino/catalog/TestCatalogManager.java | 97 +++++++++++++++++++++-
.../server/web/rest/ExceptionHandlers.java | 4 +-
.../server/web/rest/TestCatalogOperations.java | 28 +++++++
4 files changed, 151 insertions(+), 6 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
index 16b5d6a9e1..0ca99e3af8 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -65,6 +65,7 @@ import java.util.stream.Collectors;
import javax.annotation.Nullable;
import lombok.Getter;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.CatalogChange;
@@ -123,6 +124,7 @@ import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.storage.relational.SupportsEntityChangeLog;
import org.apache.gravitino.utils.ClassLoaderKey;
import org.apache.gravitino.utils.ClassLoaderPool;
+import org.apache.gravitino.utils.ExceptionMessages;
import org.apache.gravitino.utils.IsolatedClassLoader;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PooledClassLoaderEntry;
@@ -1380,9 +1382,8 @@ public class CatalogManager implements CatalogDispatcher,
Closeable {
Namespace schemaNs = Namespace.of(ident.namespace().level(0),
ident.name());
List<SchemaEntity> schemaEntities =
store.list(schemaNs, SchemaEntity.class, EntityType.SCHEMA);
- if (!force && containsUserCreatedSchemas(schemaEntities,
catalogWrapper)) {
- throw new NonEmptyCatalogException(
- "Catalog %s has schemas, please drop them first or use force
option", ident);
+ if (!force) {
+ checkCatalogSchemasBeforeDrop(ident, schemaEntities,
catalogWrapper);
}
// Child write-through secrets: capture properties (URNs) from the
entity, then
@@ -1466,6 +1467,27 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
: new HashMap<>(properties);
}
+ private void checkCatalogSchemasBeforeDrop(
+ NameIdentifier ident, List<SchemaEntity> schemaEntities, CatalogWrapper
catalogWrapper)
+ throws Exception {
+ try {
+ if (containsUserCreatedSchemas(schemaEntities, catalogWrapper)) {
+ throw new NonEmptyCatalogException(
+ "Catalog %s has schemas, please drop them first or use force
option", ident);
+ }
+ } catch (Exception e) {
+ // Some catalogs wrap connection failures, so inspect the cause chain as
well.
+ if (ExceptionUtils.indexOfType(e, ConnectionFailedException.class) >= 0)
{
+ throw new ConnectionFailedException(
+ e,
+ "%s. Use force=true to skip this check.",
+ ExceptionMessages.withCause(
+ "Failed to check schemas in the underlying catalog " + ident,
e));
+ }
+ throw e;
+ }
+ }
+
/**
* Check if the given list of schema entities contains any currently
existing user-created
* schemas.
diff --git
a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
index 6dbc3662f0..58709a12cc 100644
--- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
+++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java
@@ -66,12 +66,14 @@ import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.CatalogDropAware;
import org.apache.gravitino.connector.CatalogOperations;
import org.apache.gravitino.connector.HiddenPropertyMaskUtils;
+import org.apache.gravitino.connector.SupportsSchemas;
import org.apache.gravitino.connector.TestCatalogOperations;
import org.apache.gravitino.connector.capability.Capability;
import org.apache.gravitino.connector.capability.CapabilityResult;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogNotInUseException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -109,6 +111,8 @@ import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
@@ -1376,6 +1380,90 @@ public class TestCatalogManager {
Assertions.assertTrue(catalogManager.dropCatalog(ident));
}
+ @ParameterizedTest
+ @CsvSource({"false, false", "false, true", "true, false", "true, true"})
+ void testDropCatalogExplainsSchemaConnectionFailure(boolean failOnLoad,
boolean wrapped)
+ throws Exception {
+ NameIdentifier ident = NameIdentifier.of("metalake",
"unreachable_catalog");
+ Map<String, String> props =
+ ImmutableMap.of(
+ "provider",
+ "test",
+ PROPERTY_KEY1,
+ "value1",
+ PROPERTY_KEY2,
+ "value2",
+ PROPERTY_KEY5_PREFIX + "1",
+ "value3");
+ catalogManager.createCatalog(ident, Catalog.Type.RELATIONAL, provider,
"comment", props);
+ catalogManager.disableCatalog(ident);
+ BaseCatalog<?> catalog =
catalogManager.loadCatalogAndWrap(ident).catalog();
+
+ Namespace schemaNs = Namespace.of("metalake", ident.name());
+ SchemaEntity schemaEntity =
+ SchemaEntity.builder()
+ .withId(RandomIdGenerator.INSTANCE.nextId())
+ .withName("test_schema")
+ .withNamespace(schemaNs)
+ .withAuditInfo(
+
AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build())
+ .build();
+ entityStore.put(schemaEntity);
+
+ ConnectionFailedException connectionFailure =
+ new ConnectionFailedException(
+ new IOException("Connection refused"), "Failed to connect to Hive
Metastore");
+ RuntimeException failure =
+ wrapped
+ ? new GravitinoRuntimeException(
+ new RuntimeException(connectionFailure), "Failed to connect to
Hive Metastore")
+ : connectionFailure;
+ SupportsSchemas schemaOps = Mockito.mock(SupportsSchemas.class);
+ if (failOnLoad) {
+ Mockito.when(schemaOps.listSchemas(schemaNs))
+ .thenReturn(new NameIdentifier[] {schemaEntity.nameIdentifier()});
+
Mockito.when(schemaOps.loadSchema(schemaEntity.nameIdentifier())).thenThrow(failure);
+ } else {
+ Mockito.when(schemaOps.listSchemas(schemaNs)).thenThrow(failure);
+ }
+
+ CatalogManager.CatalogWrapper wrapper =
Mockito.mock(CatalogManager.CatalogWrapper.class);
+ Capability capability = Mockito.mock(Capability.class);
+ Mockito.when(wrapper.tryAcquire()).thenReturn(true);
+ Mockito.when(wrapper.catalog()).thenReturn(catalog);
+ Mockito.when(wrapper.capabilities()).thenReturn(capability);
+ Mockito.when(capability.managedStorage(any()))
+ .thenReturn(CapabilityResult.unsupported("Not managed"));
+ Mockito.doAnswer(
+ invocation -> {
+ ThrowableFunction<SupportsSchemas, ?> function =
invocation.getArgument(0);
+ return function.apply(schemaOps);
+ })
+ .when(wrapper)
+ .doWithSchemaOps(any());
+ catalogManager.getCatalogCache().put(ident, wrapper);
+
+ ConnectionFailedException exception =
+ Assertions.assertThrows(
+ ConnectionFailedException.class, () ->
catalogManager.dropCatalog(ident, false));
+ Assertions.assertEquals(
+ "Failed to check schemas in the underlying catalog
metalake.unreachable_catalog: "
+ + "Failed to connect to Hive Metastore: Connection refused. "
+ + "Use force=true to skip this check.",
+ exception.getMessage());
+ Assertions.assertSame(failure, exception.getCause());
+ Assertions.assertTrue(entityStore.exists(ident, EntityType.CATALOG));
+ Assertions.assertTrue(entityStore.exists(schemaEntity.nameIdentifier(),
EntityType.SCHEMA));
+ Mockito.verify(schemaOps).listSchemas(schemaNs);
+ Mockito.verify(schemaOps, Mockito.times(failOnLoad ? 1 : 0))
+ .loadSchema(schemaEntity.nameIdentifier());
+
+ Mockito.clearInvocations(schemaOps);
+ Assertions.assertTrue(catalogManager.dropCatalog(ident, true));
+ Mockito.verifyNoInteractions(schemaOps);
+ Assertions.assertFalse(entityStore.exists(ident, EntityType.CATALOG));
+ }
+
@Test
public void testDropCatalogFailsOnSchemaClassificationError() throws
Exception {
NameIdentifier ident = NameIdentifier.of("metalake", "test41");
@@ -1416,8 +1504,9 @@ public class TestCatalogManager {
Mockito.doReturn(catalog).when(wrapper).catalog();
Mockito.doReturn(capability).when(wrapper).capabilities();
Mockito.doReturn(unsupportedResult).when(capability).managedStorage(any());
+ RuntimeException failure = new RuntimeException("Failed connect");
Mockito.doReturn(new NameIdentifier[] {NameIdentifier.of("metalake",
"test41", "test_schema1")})
- .doThrow(new RuntimeException("Failed connect"))
+ .doThrow(failure)
.when(wrapper)
.doWithSchemaOps(any());
catalogManager.getCatalogCache().put(ident, wrapper);
@@ -1425,7 +1514,10 @@ public class TestCatalogManager {
// Unexpected errors during schema classification should propagate
(fail-closed).
RuntimeException ex =
Assertions.assertThrows(RuntimeException.class, () ->
catalogManager.dropCatalog(ident));
- Assertions.assertTrue(ex.getCause().getMessage().contains("Failed
connect"));
+ Assertions.assertSame(failure, ex.getCause());
+ Assertions.assertFalse(ex instanceof ConnectionFailedException);
+ Assertions.assertFalse(ex.getMessage().contains("force=true"));
+ Assertions.assertTrue(entityStore.exists(ident, EntityType.CATALOG));
}
@Test
@@ -1469,6 +1561,7 @@ public class TestCatalogManager {
.doWithSchemaOps(any());
catalogManager.getCatalogCache().put(ident, catalogWrapper);
Assertions.assertTrue(catalogManager.dropCatalog(ident, true));
+ Mockito.verify(catalogWrapper, Mockito.never()).doWithSchemaOps(any());
}
@Test
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
index 0f54d4506a..34990dc7d3 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/ExceptionHandlers.java
@@ -430,7 +430,9 @@ public class ExceptionHandlers {
@Override
public Response handle(OperationType op, String catalog, String metalake,
Exception e) {
String formatted = StringUtil.isBlank(catalog) ? "" : " [" + catalog +
"]";
- String errorMsg = getCatalogErrorMsg(formatted, op.name(), metalake,
getErrorMsg(e));
+ // Keep schema-check context even when the backend message contains
"Exception: ".
+ String reason = e instanceof ConnectionFailedException ? e.getMessage()
: getErrorMsg(e);
+ String errorMsg = getCatalogErrorMsg(formatted, op.name(), metalake,
reason);
LOG.warn(errorMsg, e);
if (e instanceof IllegalArgumentException) {
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
index b3f02a1c0c..16ac02e0d5 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
@@ -686,6 +686,34 @@ public class TestCatalogOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse.getType());
}
+ @Test
+ void testDropCatalogWithSchemaConnectionFailure() {
+ String message =
+ "Failed to check schemas in the underlying catalog metalake1.catalog1:
"
+ + "Failed to connect to Hive Metastore: TTransportException:
Connection refused. "
+ + "Use force=true to skip this check.";
+ doThrow(new ConnectionFailedException("%s", message))
+ .when(manager)
+ .dropCatalog(NameIdentifier.of("metalake1", "catalog1"), false);
+
+ try (Response response =
+ target("/metalakes/metalake1/catalogs/catalog1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .delete()) {
+ Assertions.assertEquals(Response.Status.BAD_GATEWAY.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.CONNECTION_FAILED_CODE,
error.getCode());
+ Assertions.assertEquals(ConnectionFailedException.class.getSimpleName(),
error.getType());
+ Assertions.assertEquals(
+ "Failed to operate catalog(s) [catalog1] operation [DROP] under
metalake [metalake1], "
+ + "reason ["
+ + message
+ + "]",
+ error.getMessage());
+ }
+ }
+
@Test
public void testSetCatalog() {
// Test enable catalog
