This is an automated email from the ASF dual-hosted git repository.
jshao 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 dc655c6f6 [#5329] improvement(core): Clarify exception when importing
entity multiple times (#5844)
dc655c6f6 is described below
commit dc655c6f6da4be92927abb606ab424295f749c1f
Author: mchades <[email protected]>
AuthorDate: Tue Dec 17 13:58:50 2024 +0800
[#5329] improvement(core): Clarify exception when importing entity multiple
times (#5844)
### What changes were proposed in this pull request?
- remind users not to register multiple catalogs using the same data
source in the user doc
- clarify exception when importing entity multiple times.
### Why are the changes needed?
Fix: #5329
import one source multiple times will update the existing record and
result in unexpected behaviors
### Does this PR introduce _any_ user-facing change?
yes, clarify the exception
### How was this patch tested?
tests added
---
.../integration/test/CatalogPostgreSqlIT.java | 37 ++++++++++++++++------
.../catalog/SchemaOperationDispatcher.java | 6 ++++
.../catalog/TableOperationDispatcher.java | 6 ++++
docs/manage-relational-metadata-using-gravitino.md | 5 +++
4 files changed, 44 insertions(+), 10 deletions(-)
diff --git
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
index 558775014..25f99c797 100644
---
a/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
+++
b/catalogs/catalog-jdbc-postgresql/src/test/java/org/apache/gravitino/catalog/postgresql/integration/test/CatalogPostgreSqlIT.java
@@ -118,8 +118,8 @@ public class CatalogPostgreSqlIT extends BaseIT {
postgreSqlService = new PostgreSqlService(POSTGRESQL_CONTAINER,
TEST_DB_NAME);
createMetalake();
- createCatalog();
- createSchema();
+ catalog = createCatalog(catalogName);
+ createSchema(schemaName);
}
@AfterAll
@@ -139,7 +139,7 @@ public class CatalogPostgreSqlIT extends BaseIT {
@AfterEach
public void resetSchema() {
clearTableAndSchema();
- createSchema();
+ createSchema(schemaName);
}
private void clearTableAndSchema() {
@@ -162,7 +162,7 @@ public class CatalogPostgreSqlIT extends BaseIT {
metalake = loadMetalake;
}
- private void createCatalog() throws SQLException {
+ private Catalog createCatalog(String catalogName) throws SQLException {
Map<String, String> catalogProperties = Maps.newHashMap();
String jdbcUrl = POSTGRESQL_CONTAINER.getJdbcUrl(TEST_DB_NAME);
@@ -179,10 +179,10 @@ public class CatalogPostgreSqlIT extends BaseIT {
Catalog loadCatalog = metalake.loadCatalog(catalogName);
Assertions.assertEquals(createdCatalog, loadCatalog);
- catalog = loadCatalog;
+ return loadCatalog;
}
- private void createSchema() {
+ private void createSchema(String schemaName) {
Schema createdSchema =
catalog.asSchemas().createSchema(schemaName, schema_comment,
Collections.EMPTY_MAP);
@@ -654,7 +654,7 @@ public class CatalogPostgreSqlIT extends BaseIT {
}
@Test
- void testCreateAndLoadSchema() {
+ void testCreateAndLoadSchema() throws SQLException {
String testSchemaName = "test";
Schema schema = catalog.asSchemas().createSchema(testSchemaName,
"comment", null);
@@ -665,15 +665,32 @@ public class CatalogPostgreSqlIT extends BaseIT {
Assertions.assertEquals("comment", schema.comment());
// test null comment
- testSchemaName = "test2";
+ String testSchemaName2 = "test2";
- schema = catalog.asSchemas().createSchema(testSchemaName, null, null);
+ schema = catalog.asSchemas().createSchema(testSchemaName2, null, null);
Assertions.assertEquals("anonymous", schema.auditInfo().creator());
// todo: Gravitino put id to comment, makes comment is empty string not
null.
Assertions.assertTrue(StringUtils.isEmpty(schema.comment()));
- schema = catalog.asSchemas().loadSchema(testSchemaName);
+ schema = catalog.asSchemas().loadSchema(testSchemaName2);
Assertions.assertEquals("anonymous", schema.auditInfo().creator());
Assertions.assertTrue(StringUtils.isEmpty(schema.comment()));
+
+ // test register PG service to multiple catalogs
+ String newCatalogName = GravitinoITUtils.genRandomName("new_catalog");
+ Catalog newCatalog = createCatalog(newCatalogName);
+ newCatalog.asSchemas().loadSchema(testSchemaName2);
+ Assertions.assertTrue(catalog.asSchemas().dropSchema(testSchemaName2,
false));
+ createSchema(testSchemaName2);
+ SupportsSchemas schemaOps = newCatalog.asSchemas();
+ Assertions.assertThrows(
+ UnsupportedOperationException.class, () ->
schemaOps.loadSchema(testSchemaName2));
+ // recovered by re-build the catalog
+ Assertions.assertTrue(metalake.dropCatalog(newCatalogName, true));
+ newCatalog = createCatalog(newCatalogName);
+ Schema loadedSchema = newCatalog.asSchemas().loadSchema(testSchemaName2);
+ Assertions.assertEquals(testSchemaName2, loadedSchema.name());
+
+ Assertions.assertTrue(metalake.dropCatalog(newCatalogName, true));
}
@Test
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
index ea423abfa..c6ec025ab 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/SchemaOperationDispatcher.java
@@ -24,6 +24,7 @@ import static
org.apache.gravitino.utils.NameIdentifierUtil.getCatalogIdentifier
import java.time.Instant;
import java.util.Map;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Namespace;
@@ -350,6 +351,11 @@ public class SchemaOperationDispatcher extends
OperationDispatcher implements Sc
.build();
try {
store.put(schemaEntity, true);
+ } catch (EntityAlreadyExistsException e) {
+ LOG.error("Failed to import schema {} with id {} to the store.",
identifier, uid, e);
+ throw new UnsupportedOperationException(
+ "Schema managed by multiple catalogs. This may cause unexpected
issues such as privilege conflicts. "
+ + "To resolve: Remove all catalogs managing this schema, then
recreate one catalog to ensure single-catalog management.");
} catch (Exception e) {
LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "put", identifier, e);
throw new RuntimeException("Fail to import schema entity to the store.",
e);
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
index da869d65f..de34e712a 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java
@@ -35,6 +35,7 @@ import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.apache.commons.lang3.tuple.Pair;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
@@ -394,6 +395,11 @@ public class TableOperationDispatcher extends
OperationDispatcher implements Tab
.build();
try {
store.put(tableEntity, true);
+ } catch (EntityAlreadyExistsException e) {
+ LOG.error("Failed to import table {} with id {} to the store.",
identifier, uid, e);
+ throw new UnsupportedOperationException(
+ "Table managed by multiple catalogs. This may cause unexpected
issues such as privilege conflicts. "
+ + "To resolve: Remove all catalogs managing this table, then
recreate one catalog to ensure single-catalog management.");
} catch (Exception e) {
LOG.error(FormattedErrorMessages.STORE_OP_FAILURE, "put", identifier, e);
throw new RuntimeException("Fail to import the table entity to the
store.", e);
diff --git a/docs/manage-relational-metadata-using-gravitino.md
b/docs/manage-relational-metadata-using-gravitino.md
index 280793e69..352a8de29 100644
--- a/docs/manage-relational-metadata-using-gravitino.md
+++ b/docs/manage-relational-metadata-using-gravitino.md
@@ -36,6 +36,11 @@ Assuming:
### Create a catalog
+:::caution
+It is not recommended to use one data source to create multiple catalogs,
+as multiple catalogs operating on the same source may result in unpredictable
behavior.
+:::
+
:::tip
The code below is an example of creating a Hive catalog. For other relational
catalogs, the code is
similar, but the catalog type, provider, and properties may be different. For
more details, please refer to the related doc.
