This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-9223348f-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit f61c65b9e395c0979b798df332d87b5c829b9af3 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. # Conflicts: # core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java # core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java --- .../apache/gravitino/catalog/CatalogManager.java | 42 ++++++++- .../gravitino/catalog/TestCatalogManager.java | 101 ++++++++++++++++++++- .../server/web/rest/ExceptionHandlers.java | 4 +- .../server/web/rest/TestCatalogOperations.java | 28 ++++++ 4 files changed, 169 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 5c25e4521e..2ef99bf06c 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; @@ -113,6 +114,12 @@ import org.apache.gravitino.rel.TableCatalog; import org.apache.gravitino.rel.ViewCatalog; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.relational.SupportsEntityChangeLog; +<<<<<<< HEAD +======= +import org.apache.gravitino.utils.ClassLoaderKey; +import org.apache.gravitino.utils.ClassLoaderPool; +import org.apache.gravitino.utils.ExceptionMessages; +>>>>>>> 9223348fc ([#13809] fix(core): Clarify catalog drop errors for unreachable backends (#13810)) import org.apache.gravitino.utils.IsolatedClassLoader; import org.apache.gravitino.utils.NamespaceUtil; import org.apache.gravitino.utils.PrincipalUtils; @@ -1284,9 +1291,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); } if (isManagedStorageCatalog(catalogWrapper)) { @@ -1337,6 +1343,36 @@ public class CatalogManager implements CatalogDispatcher, Closeable { }); } +<<<<<<< HEAD +======= + private static Map<String, String> copyProperties(Map<String, String> properties) { + return properties == null || properties.isEmpty() + ? Collections.emptyMap() + : 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; + } + } + +>>>>>>> 9223348fc ([#13809] fix(core): Clarify catalog drop errors for unreachable backends (#13810)) /** * 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 b415f35bd7..8531e9a23c 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java @@ -60,12 +60,18 @@ import org.apache.gravitino.Schema; import org.apache.gravitino.connector.BaseCatalog; import org.apache.gravitino.connector.CatalogDropAware; import org.apache.gravitino.connector.CatalogOperations; +<<<<<<< HEAD +======= +import org.apache.gravitino.connector.HiddenPropertyMaskUtils; +import org.apache.gravitino.connector.SupportsSchemas; +>>>>>>> 9223348fc ([#13809] fix(core): Clarify catalog drop errors for unreachable backends (#13810)) 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; @@ -93,6 +99,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; @@ -1116,6 +1124,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"); @@ -1156,8 +1248,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); @@ -1165,7 +1258,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 @@ -1209,6 +1305,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 a6e3e38a43..36791cf1ba 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 @@ -409,7 +409,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 54b91fdad8..3d2c697502 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 @@ -685,6 +685,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
