diff --git a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java index d8a4f77c130..53625230a35 100644 --- a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java +++ b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java @@ -59,6 +59,7 @@ import java.io.IOException; import java.net.ConnectException; import java.nio.file.Paths; +import java.time.Instant; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -104,6 +105,10 @@ import org.apache.gravitino.file.FileInfo; import org.apache.gravitino.file.Fileset; import org.apache.gravitino.file.FilesetChange; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.secret.SecretConstants; import org.apache.gravitino.secret.SecretManager; import org.apache.gravitino.secret.SecretMaterial; @@ -231,7 +236,7 @@ private static CatalogInfo randomCatalogInfo( } @BeforeAll - public static void setUp() throws IllegalAccessException { + public static void setUp() throws IOException, IllegalAccessException { Config config = Mockito.mock(Config.class); when(config.get(ENTITY_STORE)).thenReturn(RELATIONAL_ENTITY_STORE); when(config.get(ENTITY_RELATIONAL_STORE)).thenReturn(DEFAULT_ENTITY_RELATIONAL_STORE); @@ -273,6 +278,28 @@ public static void setUp() throws IllegalAccessException { store.initialize(config); idGenerator = new RandomIdGenerator(); + AuditInfo auditInfo = + AuditInfo.builder().withCreator("test").withCreateTime(Instant.now()).build(); + BaseMetalake metalake = + BaseMetalake.builder() + .withId(1L) + .withName("m1") + .withVersion(SchemaVersion.V_0_1) + .withAuditInfo(auditInfo) + .build(); + store.put(metalake, false); + + CatalogEntity catalog = + CatalogEntity.builder() + .withId(1L) + .withName("c1") + .withNamespace(Namespace.of("m1")) + .withProvider("fileset") + .withType(Catalog.Type.FILESET) + .withAuditInfo(auditInfo) + .build(); + store.put(catalog, false); + // Mock MetalakeMetaService metalakeMetaService = MetalakeMetaService.getInstance(); MetalakeMetaService spyMetaService = Mockito.spy(metalakeMetaService); @@ -428,14 +455,13 @@ public void testCreateSchemaWithNoLocation() throws IOException { final long testId = generateTestId(); final String name = "schema" + testId; final String comment = "comment" + testId; - Schema schema = createSchema(testId, name, comment, null, null); + Schema schema = createSchema(name, comment, null, null); Assertions.assertEquals(name, schema.name()); Assertions.assertEquals(comment, schema.comment()); Throwable exception = Assertions.assertThrows( - SchemaAlreadyExistsException.class, - () -> createSchema(testId, name, comment, null, null)); + SchemaAlreadyExistsException.class, () -> createSchema(name, comment, null, null)); Assertions.assertEquals( "Schema m1.c1.schema" + testId + " already exists", exception.getMessage()); } @@ -450,7 +476,7 @@ public void testCreateSchemaWithEmptyCatalogLocation() throws IOException { Throwable exception = Assertions.assertThrows( IllegalArgumentException.class, - () -> createSchema(testId, schemaName, comment, catalogPath, null)); + () -> createSchema(schemaName, comment, catalogPath, null)); Assertions.assertEquals( "The value of the catalog property " + FilesetCatalogPropertiesMetadata.LOCATION @@ -464,7 +490,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { String name = "schema" + testId; final String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog12"; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); Path schemaPath = new Path(catalogPath, name); @@ -476,7 +502,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { // test placeholder in catalog location name = "schema" + testId + "_1"; catalogPath = TEST_ROOT_PATH + "/" + "{{catalog}}-{{schema}}"; - schema = createSchema(testId, name, comment, catalogPath, null); + schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); schemaPath = new Path(catalogPath, name); @@ -486,7 +512,7 @@ public void testCreateSchemaWithCatalogLocation() throws IOException { // Test disable server-side FS operations. name = "schema" + testId + "_2"; catalogPath = TEST_ROOT_PATH + "/" + "catalog12_2"; - schema = createSchema(testId, name, comment, catalogPath, null, true); + schema = createSchema(name, comment, catalogPath, null, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -501,7 +527,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { final String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; String schemaPath = catalogPath + "/" + name; - Schema schema = createSchema(testId, name, comment, null, schemaPath); + Schema schema = createSchema(name, comment, null, schemaPath); Assertions.assertEquals(name, schema.name()); Path schemaPath1 = new Path(schemaPath); @@ -513,7 +539,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { // test placeholder in schema location name = "schema" + testId + "_1"; schemaPath = catalogPath + "/" + "{{schema}}"; - schema = createSchema(testId, name, comment, null, schemaPath); + schema = createSchema(name, comment, null, schemaPath); Assertions.assertEquals(name, schema.name()); schemaPath1 = new Path(schemaPath); @@ -526,7 +552,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { Throwable exception = Assertions.assertThrows( IllegalArgumentException.class, - () -> createSchema(testId, schemaName1, comment, null, schemaPath2)); + () -> createSchema(schemaName1, comment, null, schemaPath2)); Assertions.assertTrue( exception.getMessage().contains("Placeholder in location should not be empty"), exception.getMessage()); @@ -534,7 +560,7 @@ public void testCreateSchemaWithSchemaLocation() throws IOException { // Test disable server-side FS operations. name = "schema" + testId + "_3"; schemaPath = catalogPath + "/" + name; - schema = createSchema(testId, name, comment, null, schemaPath, true); + schema = createSchema(name, comment, null, schemaPath, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -548,7 +574,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; String schemaPath = TEST_ROOT_PATH + "/" + "schema" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, schemaPath); + Schema schema = createSchema(name, comment, catalogPath, schemaPath); Assertions.assertEquals(name, schema.name()); Path schemaPath1 = new Path(schemaPath); @@ -564,7 +590,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { name = "schema" + testId + "_1"; catalogPath = TEST_ROOT_PATH + "/" + "{{catalog}}"; schemaPath = TEST_ROOT_PATH + "/" + "{{schema}}"; - schema = createSchema(testId, name, comment, catalogPath, schemaPath); + schema = createSchema(name, comment, catalogPath, schemaPath); Assertions.assertEquals(name, schema.name()); schemaPath1 = new Path(schemaPath); @@ -577,7 +603,7 @@ public void testCreateSchemaWithCatalogAndSchemaLocation() throws IOException { name = "schema" + testId + "_2"; catalogPath = TEST_ROOT_PATH + "/" + "catalog14_2"; schemaPath = TEST_ROOT_PATH + "/" + "schema14_2"; - schema = createSchema(testId, name, comment, catalogPath, schemaPath, true); + schema = createSchema(name, comment, catalogPath, schemaPath, true); Assertions.assertEquals(name, schema.name()); // Schema path should not be existed if the server-side FS operations are disabled. @@ -591,7 +617,7 @@ public void testLoadSchema() throws IOException { String name = "schema" + testId; String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); NameIdentifier otherSchema = NameIdentifierUtil.ofSchema("m1", "c1", "otherSchema"); Assertions.assertEquals(name, schema.name()); @@ -619,8 +645,8 @@ public void testListSchema() throws IOException { String comment1 = "comment" + testId1; String name2 = "schema" + testId2; String comment2 = "comment" + testId2; - createSchema(testId1, name1, comment1, null, null); - createSchema(testId2, name2, comment2, null, null); + createSchema(name1, comment1, null, null); + createSchema(name2, comment2, null, null); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { ops.initialize(Maps.newHashMap(), randomCatalogInfo(), FILESET_PROPERTIES_METADATA); @@ -638,7 +664,7 @@ public void testAlterSchema() throws IOException { String name = "schema" + testId; String comment = "comment" + testId; String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, name, comment, catalogPath, null); + Schema schema = createSchema(name, comment, catalogPath, null); Assertions.assertEquals(name, schema.name()); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { @@ -686,7 +712,7 @@ public void testDropSchema() throws IOException { final String comment = "comment" + testId; final String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, schemaName, comment, catalogPath, null); + Schema schema = createSchema(schemaName, comment, catalogPath, null); Assertions.assertEquals(schemaName, schema.name()); NameIdentifier id = NameIdentifierUtil.ofSchema("m1", "c1", schemaName); @@ -709,7 +735,7 @@ public void testDropSchema() throws IOException { Assertions.assertFalse(fs.exists(schemaPath)); // Test drop non-empty schema with cascade = false - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs1 = createFileset("fs1", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, null); Path fs1Path = new Path(fs1.storageLocation()); @@ -725,7 +751,7 @@ public void testDropSchema() throws IOException { Assertions.assertFalse(fs.exists(fs1Path)); // Test drop both managed and external filesets - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs2 = createFileset("fs2", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, null); Path fs2Path = new Path(fs2.storageLocation()); @@ -741,7 +767,7 @@ public void testDropSchema() throws IOException { Assertions.assertTrue(fs.exists(fs3Path)); // Test drop schema with different storage location - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Path fs4Path = new Path(TEST_ROOT_PATH + "/fs4"); createFileset( "fs4", schemaName, "comment", Fileset.Type.MANAGED, catalogPath, fs4Path.toString()); @@ -758,7 +784,7 @@ public void testDropSchemaWithFSOpsDisabled() throws IOException { final String filesetName = "fileset" + testId; final String catalogPath = TEST_ROOT_PATH + "/" + "catalog" + testId; - Schema schema = createSchema(testId, schemaName, comment, catalogPath, null); + Schema schema = createSchema(schemaName, comment, catalogPath, null); Assertions.assertEquals(schemaName, schema.name()); NameIdentifier id = NameIdentifierUtil.ofSchema("m1", "c1", schemaName); @@ -774,7 +800,7 @@ public void testDropSchemaWithFSOpsDisabled() throws IOException { FileSystem fs = schemaPath.getFileSystem(new Configuration()); Assertions.assertTrue(fs.exists(schemaPath)); - createSchema(testId, schemaName, comment, catalogPath, null); + createSchema(schemaName, comment, catalogPath, null); Fileset fs1 = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, catalogPath, null); Path fs1Path = new Path(fs1.storageLocation()); @@ -807,7 +833,7 @@ public void testCreateLoadAndDeleteFilesetWithLocations( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset(name, schemaName, "comment", type, catalogPath, storageLocation); @@ -864,7 +890,7 @@ public void testCreateLoadAndDeleteFilesetWithLocationsWhenFSOpsDisabled( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath, true); + createSchema(schemaName, comment, catalogPath, schemaPath, true); } Fileset fileset; @@ -922,7 +948,7 @@ public void testCreateFilesetWithExceptions() throws IOException { final String comment = "comment" + testId; final String filesetName = "fileset" + testId; - createSchema(testId, schemaName, comment, null, null); + createSchema(schemaName, comment, null, null); NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); // If neither catalog location, nor schema location and storageLocation is specified. @@ -972,7 +998,7 @@ public void testListFilesets() throws IOException { String comment = "comment" + testId; String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); String[] filesets = { "fileset" + testId + "_1", "fileset" + testId + "_2", "fileset" + testId + "_3" @@ -1002,7 +1028,7 @@ public void testListFilesetFiles() throws IOException { final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; final NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { @@ -1052,7 +1078,7 @@ public void testListFilesetFilesWithFSOpsDisabled() throws Exception { final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; final NameIdentifier filesetIdent = NameIdentifier.of("m1", "c1", schemaName, filesetName); - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); Map catalogProps = Collections.singletonMap(DISABLE_FILESYSTEM_OPS, "true"); @@ -1079,7 +1105,7 @@ public void testListFilesetFilesWithNonExistentPath() throws IOException { String filesetName = "fileset" + testId; final String nonExistentSubPath = "/non_existent_file.txt"; - Schema schema = createSchema(testId, schemaName, comment, null, schemaPath); + Schema schema = createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); final NameIdentifier filesetIdent = @@ -1121,7 +1147,7 @@ public void testRenameFileset( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset(name, schemaName, "comment", type, catalogPath, storageLocation); @@ -1159,7 +1185,7 @@ public void testAlterFilesetProperties() throws IOException { final String filesetName = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); @@ -1274,7 +1300,7 @@ public void testUpdateFilesetComment() throws IOException { final String name = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(name, schemaName, comment, Fileset.Type.MANAGED, null, null); FilesetChange change1 = FilesetChange.updateComment(comment + "_new"); @@ -1298,7 +1324,7 @@ public void testRemoveFilesetComment() throws IOException { final String filesetName = "fileset" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset(filesetName, schemaName, comment, Fileset.Type.MANAGED, null, null); @@ -1372,7 +1398,7 @@ public void testGetFileLocation() throws IOException { final String storageLocation = TEST_ROOT_PATH + "/" + catalogName + "/" + schemaName + "/" + filesetName; - createSchema(testId, schemaName, comment, null, schemaPath); + createSchema(schemaName, comment, null, schemaPath); Fileset fileset = createFileset( filesetName, schemaName, comment, Fileset.Type.MANAGED, null, storageLocation); @@ -1536,7 +1562,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String schemaName = "schema" + testId; final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByAlice); @@ -1552,7 +1578,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; // Create schema with user "bob" - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByBob); Assertions.assertEquals("bob", schemaCreatedByBob.auditInfo().creator()); @@ -1567,7 +1593,7 @@ public void testCreateSchemaWithDifferentUser() throws Exception { final String comment = "comment" + testId; final String schemaPath = TEST_ROOT_PATH + "/" + schemaName; // Create schema with user "lucy" - return createSchema(testId, schemaName, comment, null, schemaPath, false); + return createSchema(schemaName, comment, null, schemaPath, false); }); Assertions.assertNotNull(schemaCreatedByLucy); Assertions.assertEquals("lucy", schemaCreatedByLucy.auditInfo().creator()); @@ -1581,7 +1607,7 @@ public void testLocationPlaceholdersWithException() throws IOException { final String filesetName = "fileset" + testId; String storageLocation = TEST_ROOT_PATH + "/{{fileset}}/{{user}}/{{id}}"; - createSchema(testId, schemaName, null, null, null); + createSchema(schemaName, null, null, null); Exception exception = Assertions.assertThrows( @@ -1638,7 +1664,7 @@ public void testPlaceholdersInLocation( try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { ops.initialize(catalogProps, randomCatalogInfo("m1", "c1"), FILESET_PROPERTIES_METADATA); if (!ops.schemaExists(schemaIdent)) { - createSchema(generateTestId(), schemaName, comment, catalogPath, schemaPath); + createSchema(schemaName, comment, catalogPath, schemaPath); } Fileset fileset = createFileset( @@ -2953,22 +2979,17 @@ private static Stream testRenameArguments() { TEST_ROOT_PATH + "/fileset39")); } - private Schema createSchema( - long testId, String name, String comment, String catalogPath, String schemaPath) + private Schema createSchema(String name, String comment, String catalogPath, String schemaPath) throws IOException { - return createSchema(testId, name, comment, catalogPath, schemaPath, false); + return createSchema(name, comment, catalogPath, schemaPath, false); } private Schema createSchema( - long testId, - String name, - String comment, - String catalogPath, - String schemaPath, - boolean disableFsOps) + String name, String comment, String catalogPath, String schemaPath, boolean disableFsOps) throws IOException { + long schemaId = idGenerator.nextId(); // stub schema - doReturn(new SchemaIds(1L, 1L, testId)) + doReturn(new SchemaIds(1L, 1L, schemaId)) .when(spySchemaMetaService) .getSchemaIdByMetalakeNameAndCatalogNameAndSchemaName( Mockito.anyString(), Mockito.anyString(), Mockito.eq(name)); @@ -2984,7 +3005,7 @@ private Schema createSchema( NameIdentifier schemaIdent = NameIdentifierUtil.ofSchema("m1", "c1", name); Map schemaProps = Maps.newHashMap(); - StringIdentifier stringId = StringIdentifier.fromId(testId); + StringIdentifier stringId = StringIdentifier.fromId(schemaId); schemaProps = Maps.newHashMap(StringIdentifier.newPropertiesWithId(stringId, schemaProps)); if (schemaPath != null) { diff --git a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java index dd30df96f3e..69fa82cacdf 100644 --- a/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java +++ b/catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/TestKafkaCatalogOperations.java @@ -46,7 +46,6 @@ import static org.apache.gravitino.catalog.kafka.KafkaCatalogPropertiesMetadata.BOOTSTRAP_SERVERS; import static org.apache.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.PARTITION_COUNT; import static org.apache.gravitino.catalog.kafka.KafkaTopicPropertiesMetadata.REPLICATION_FACTOR; -import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; @@ -74,18 +73,16 @@ import org.apache.gravitino.messaging.Topic; import org.apache.gravitino.messaging.TopicChange; import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.RandomIdGenerator; -import org.apache.gravitino.storage.relational.helper.CatalogIds; -import org.apache.gravitino.storage.relational.service.CatalogMetaService; -import org.apache.gravitino.storage.relational.service.MetalakeMetaService; import org.apache.kafka.common.config.TopicConfig; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.mockito.MockedStatic; import org.mockito.Mockito; public class TestKafkaCatalogOperations extends KafkaClusterEmbedded { @@ -141,7 +138,7 @@ public PropertiesMetadata modelVersionPropertiesMetadata() private static KafkaCatalogOperations kafkaCatalogOperations; @BeforeAll - public static void setUp() throws IllegalAccessException { + public static void setUp() throws IOException, IllegalAccessException { Config config = Mockito.mock(Config.class); Mockito.when(config.get(STORE_TRANSACTION_MAX_SKEW_TIME)).thenReturn(1000L); Mockito.when(config.get(STORE_DELETE_AFTER_TIME)).thenReturn(20 * 60 * 1000L); @@ -178,35 +175,23 @@ public static void setUp() throws IllegalAccessException { Mockito.when(config.get(Configs.CACHE_IMPLEMENTATION)).thenReturn("caffeine"); Mockito.when(config.get(Configs.CACHE_LOCK_SEGMENTS)).thenReturn(16); - // Mock - MetalakeMetaService metalakeMetaService = MetalakeMetaService.getInstance(); - MetalakeMetaService spyMetaservice = Mockito.spy(metalakeMetaService); - doReturn(1L).when(spyMetaservice).getMetalakeIdByName(Mockito.anyString()); - - CatalogMetaService catalogMetaService = CatalogMetaService.getInstance(); - CatalogMetaService spyCatalogMetaService = Mockito.spy(catalogMetaService); - doReturn(1L) - .when(spyCatalogMetaService) - .getCatalogIdByMetalakeIdAndName(Mockito.anyLong(), Mockito.anyString()); - doReturn(new CatalogIds(1L, 1L)) - .when(spyCatalogMetaService) - .getCatalogIdByMetalakeAndCatalogName(Mockito.anyString(), Mockito.anyString()); - - MockedStatic metalakeMetaServiceMockedStatic = - Mockito.mockStatic(MetalakeMetaService.class); - MockedStatic catalogMetaServiceMockedStatic = - Mockito.mockStatic(CatalogMetaService.class); - - metalakeMetaServiceMockedStatic - .when(MetalakeMetaService::getInstance) - .thenReturn(spyMetaservice); - catalogMetaServiceMockedStatic - .when(CatalogMetaService::getInstance) - .thenReturn(spyCatalogMetaService); - store = EntityStoreFactory.createEntityStore(config); store.initialize(config); idGenerator = new RandomIdGenerator(); + + BaseMetalake metalake = + BaseMetalake.builder() + .withId(1L) + .withName(METALAKE_NAME) + .withVersion(SchemaVersion.V_0_1) + .withAuditInfo( + AuditInfo.builder() + .withCreator("testKafkaUser") + .withCreateTime(Instant.now()) + .build()) + .build(); + store.put(metalake, false); + kafkaCatalogEntity = CatalogEntity.builder() .withId(1L) @@ -221,6 +206,7 @@ public static void setUp() throws IllegalAccessException { .withCreateTime(Instant.now()) .build()) .build(); + store.put(kafkaCatalogEntity, false); FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); @@ -238,11 +224,11 @@ public static void tearDown() throws IOException { } @Test - public void testKafkaCatalogConfiguration() { + public void testKafkaCatalogConfiguration() throws IOException { String catalogName = "test_kafka_catalog_configuration"; CatalogEntity catalogEntity = CatalogEntity.builder() - .withId(2L) + .withId(idGenerator.nextId()) .withName(catalogName) .withNamespace(Namespace.of(METALAKE_NAME)) .withType(MESSAGING) @@ -254,6 +240,7 @@ public void testKafkaCatalogConfiguration() { .build()) .withProperties(MOCK_CATALOG_PROPERTIES) .build(); + store.put(catalogEntity, false); KafkaCatalogOperations ops = new KafkaCatalogOperations(store, idGenerator); Assertions.assertNull(ops.adminClientConfig); @@ -274,11 +261,11 @@ public void testKafkaCatalogConfiguration() { } @Test - public void testInitialization() { + public void testInitialization() throws IOException { String catalogName = "test_kafka_catalog_initialization"; CatalogEntity catalogEntity = CatalogEntity.builder() - .withId(2L) + .withId(idGenerator.nextId()) .withName(catalogName) .withNamespace(Namespace.of(METALAKE_NAME)) .withType(MESSAGING) @@ -290,6 +277,7 @@ public void testInitialization() { .build()) .withProperties(MOCK_CATALOG_PROPERTIES) .build(); + store.put(catalogEntity, false); KafkaCatalogOperations ops = new KafkaCatalogOperations(store, idGenerator); ops.initialize( MOCK_CATALOG_PROPERTIES, catalogEntity.toCatalogInfo(), KAFKA_PROPERTIES_METADATA); 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 72c0139cf43..158d5677579 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java +++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java @@ -984,6 +984,12 @@ public boolean dropCatalog(NameIdentifier ident, boolean force) } catch (NoSuchMetalakeException | NoSuchCatalogException ignored) { return false; + } catch (NoSuchEntityException ignored) { + // Another server may have deleted the catalog after it was loaded but before this + // transaction reached the compare-and-set delete. Preserve the idempotent drop + // contract and discard the now-stale local cache entry. + catalogCache.invalidate(ident); + return false; } catch (GravitinoRuntimeException e) { throw e; } catch (Exception e) { diff --git a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java index 164d4b53460..aca4eec619a 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java +++ b/core/src/main/java/org/apache/gravitino/catalog/ManagedSchemaOperations.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Map; import org.apache.gravitino.Entity; +import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.EntityStore; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; @@ -116,9 +117,11 @@ public Schema createSchema(NameIdentifier ident, String comment, Map listCatalogPOsByMetalakeId(@Param("metalakeId") Long metalakeId); + /** Selects and locks all active catalogs in a metalake for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "listCatalogPOsByMetalakeIdForUpdate") + List listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId); + @SelectProvider(type = CatalogMetaSQLProviderFactory.class, method = "listCatalogPOsByCatalogIds") List listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds); @@ -73,6 +79,18 @@ CatalogPO selectCatalogMetaByName( @SelectProvider(type = CatalogMetaSQLProviderFactory.class, method = "selectCatalogMetaById") CatalogPO selectCatalogMetaById(@Param("catalogId") Long catalogId); + /** Selects and locks an active catalog by ID for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "selectCatalogMetaByIdForUpdate") + CatalogPO selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId); + + /** Selects and share-locks an active catalog by ID for the current transaction. */ + @SelectProvider( + type = CatalogMetaSQLProviderFactory.class, + method = "selectCatalogMetaByIdForShare") + CatalogPO selectCatalogMetaByIdForShare(@Param("catalogId") Long catalogId); + @InsertProvider(type = CatalogMetaSQLProviderFactory.class, method = "insertCatalogMeta") void insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO); @@ -89,12 +107,18 @@ Integer updateCatalogMeta( @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, method = "softDeleteCatalogMetasByCatalogId") - Integer softDeleteCatalogMetasByCatalogId(@Param("catalogId") Long catalogId); - + Integer softDeleteCatalogMetasByCatalogId( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion); + + /** + * Soft-deletes catalogs whose identifiers and OCC versions still match. + * + * @return the number of deleted rows + */ @UpdateProvider( type = CatalogMetaSQLProviderFactory.class, - method = "softDeleteCatalogMetasByMetalakeId") - Integer softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId); + method = "softDeleteCatalogMetasWithVersion") + Integer softDeleteCatalogMetasWithVersion(@Param("catalogMetas") List catalogPOs); @DeleteProvider( type = CatalogMetaSQLProviderFactory.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java index c3a7954a25a..2bc9c594318 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/CatalogMetaSQLProviderFactory.java @@ -51,7 +51,13 @@ public static CatalogMetaBaseSQLProvider getProvider() { static class CatalogMetaMySQLProvider extends CatalogMetaBaseSQLProvider {} - static class CatalogMetaH2Provider extends CatalogMetaBaseSQLProvider {} + static class CatalogMetaH2Provider extends CatalogMetaBaseSQLProvider { + @Override + public String selectCatalogMetaByIdForShare(Long catalogId) { + // H2 has no shared row-lock syntax, so use an exclusive lock in tests. + return selectCatalogMetaByIdForUpdate(catalogId); + } + } public static String listCatalogPOsByMetalakeName(@Param("metalakeName") String metalakeName) { return getProvider().listCatalogPOsByMetalakeName(metalakeName); @@ -61,6 +67,11 @@ public static String listCatalogPOsByMetalakeId(@Param("metalakeId") Long metala return getProvider().listCatalogPOsByMetalakeId(metalakeId); } + /** Returns SQL that lists and locks all active catalogs in a metalake. */ + public static String listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId) { + return getProvider().listCatalogPOsByMetalakeIdForUpdate(metalakeId); + } + public static String listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds) { return getProvider().listCatalogPOsByCatalogIds(catalogIds); } @@ -94,6 +105,16 @@ public static String selectCatalogMetaById(@Param("catalogId") Long catalogId) { return getProvider().selectCatalogMetaById(catalogId); } + /** Returns SQL that selects and locks an active catalog by ID. */ + public static String selectCatalogMetaByIdForUpdate(@Param("catalogId") Long catalogId) { + return getProvider().selectCatalogMetaByIdForUpdate(catalogId); + } + + /** Returns SQL that selects and share-locks an active catalog by ID. */ + public static String selectCatalogMetaByIdForShare(@Param("catalogId") Long catalogId) { + return getProvider().selectCatalogMetaByIdForShare(catalogId); + } + public static String insertCatalogMeta(@Param("catalogMeta") CatalogPO catalogPO) { return getProvider().insertCatalogMeta(catalogPO); } @@ -109,12 +130,15 @@ public static String updateCatalogMeta( return getProvider().updateCatalogMeta(newCatalogPO, oldCatalogPO); } - public static String softDeleteCatalogMetasByCatalogId(@Param("catalogId") Long catalogId) { - return getProvider().softDeleteCatalogMetasByCatalogId(catalogId); + public static String softDeleteCatalogMetasByCatalogId( + @Param("catalogId") Long catalogId, @Param("currentVersion") Long currentVersion) { + return getProvider().softDeleteCatalogMetasByCatalogId(catalogId, currentVersion); } - public static String softDeleteCatalogMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { - return getProvider().softDeleteCatalogMetasByMetalakeId(metalakeId); + /** Returns SQL that soft-deletes catalogs using identifier-and-version pairs. */ + public static String softDeleteCatalogMetasWithVersion( + @Param("catalogMetas") List catalogPOs) { + return getProvider().softDeleteCatalogMetasWithVersion(catalogPOs); } public static String deleteCatalogMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java index f705c283ce6..6479229aa3d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaMapper.java @@ -47,6 +47,18 @@ public interface MetalakeMetaMapper { @SelectProvider(type = MetalakeMetaSQLProviderFactory.class, method = "selectMetalakeMetaById") MetalakePO selectMetalakeMetaById(@Param("metalakeId") Long metalakeId); + /** Selects and locks an active metalake by ID for the current transaction. */ + @SelectProvider( + type = MetalakeMetaSQLProviderFactory.class, + method = "selectMetalakeMetaByIdForUpdate") + MetalakePO selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId); + + /** Selects and share-locks an active metalake by ID for the current transaction. */ + @SelectProvider( + type = MetalakeMetaSQLProviderFactory.class, + method = "selectMetalakeMetaByIdForShare") + MetalakePO selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId); + @SelectProvider( type = MetalakeMetaSQLProviderFactory.class, method = "listMetalakePOsByMetalakeIds") @@ -73,7 +85,8 @@ Integer updateMetalakeMeta( @UpdateProvider( type = MetalakeMetaSQLProviderFactory.class, method = "softDeleteMetalakeMetaByMetalakeId") - Integer softDeleteMetalakeMetaByMetalakeId(@Param("metalakeId") Long metalakeId); + Integer softDeleteMetalakeMetaByMetalakeId( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion); @DeleteProvider( type = MetalakeMetaSQLProviderFactory.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java index eba26f9e025..2a69fcd6273 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/MetalakeMetaSQLProviderFactory.java @@ -51,7 +51,13 @@ public static MetalakeMetaBaseSQLProvider getProvider() { static class MetalakeMetaMySQLProvider extends MetalakeMetaBaseSQLProvider {} - static class MetalakeMetaH2Provider extends MetalakeMetaBaseSQLProvider {} + static class MetalakeMetaH2Provider extends MetalakeMetaBaseSQLProvider { + @Override + public String selectMetalakeMetaByIdForShare(Long metalakeId) { + // H2 has no shared row-lock syntax, so use an exclusive lock in tests. + return selectMetalakeMetaByIdForUpdate(metalakeId); + } + } public String listMetalakePOs() { return getProvider().listMetalakePOs(); @@ -65,6 +71,16 @@ public static String selectMetalakeMetaById(@Param("metalakeId") Long metalakeId return getProvider().selectMetalakeMetaById(metalakeId); } + /** Returns SQL that selects and locks an active metalake by ID. */ + public static String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId) { + return getProvider().selectMetalakeMetaByIdForUpdate(metalakeId); + } + + /** Returns SQL that selects and share-locks an active metalake by ID. */ + public static String selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId) { + return getProvider().selectMetalakeMetaByIdForShare(metalakeId); + } + public static String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return getProvider().selectMetalakeIdMetaByName(metalakeName); } @@ -88,8 +104,9 @@ public static String updateMetalakeMeta( return getProvider().updateMetalakeMeta(newMetalakePO, oldMetalakePO); } - public static String softDeleteMetalakeMetaByMetalakeId(@Param("metalakeId") Long metalakeId) { - return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId); + public static String softDeleteMetalakeMetaByMetalakeId( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { + return getProvider().softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion); } public static String deleteMetalakeMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java index 1c9b5286b29..1f989615f35 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaMapper.java @@ -42,6 +42,10 @@ public interface SchemaMetaMapper { @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByCatalogId") List listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId); + /** Lists all active schemas in a metalake. */ + @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByMetalakeId") + List listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId); + @SelectProvider( type = SchemaMetaSQLProviderFactory.class, method = "listSchemaPOsByFullQualifiedName") @@ -82,6 +86,18 @@ SchemaPO selectSchemaByFullQualifiedName( @SelectProvider(type = SchemaMetaSQLProviderFactory.class, method = "selectSchemaMetaById") SchemaPO selectSchemaMetaById(@Param("schemaId") Long schemaId); + /** Selects and locks an active schema by ID for the current transaction. */ + @SelectProvider( + type = SchemaMetaSQLProviderFactory.class, + method = "selectSchemaMetaByIdForUpdate") + SchemaPO selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId); + + /** Selects and share-locks an active schema by ID for the current transaction. */ + @SelectProvider( + type = SchemaMetaSQLProviderFactory.class, + method = "selectSchemaMetaByIdForShare") + SchemaPO selectSchemaMetaByIdForShare(@Param("schemaId") Long schemaId); + @InsertProvider(type = SchemaMetaSQLProviderFactory.class, method = "insertSchemaMeta") void insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO); @@ -109,13 +125,19 @@ Integer updateSchemaMeta( @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, - method = "softDeleteSchemaMetasByMetalakeId") - Integer softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId); - + method = "softDeleteSchemaMetaBySchemaIdAndVersion") + Integer softDeleteSchemaMetaBySchemaIdAndVersion( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion); + + /** + * Soft-deletes schemas whose identifiers and OCC versions still match. + * + * @return the number of deleted rows + */ @UpdateProvider( type = SchemaMetaSQLProviderFactory.class, - method = "softDeleteSchemaMetasByCatalogId") - Integer softDeleteSchemaMetasByCatalogId(@Param("catalogId") Long catalogId); + method = "softDeleteSchemaMetasWithVersion") + Integer softDeleteSchemaMetasWithVersion(@Param("schemaMetas") List schemaPOs); @DeleteProvider( type = SchemaMetaSQLProviderFactory.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java index acc27170269..0b5c0d9c721 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/SchemaMetaSQLProviderFactory.java @@ -49,7 +49,13 @@ public static SchemaMetaBaseSQLProvider getProvider() { static class SchemaMetaMySQLProvider extends SchemaMetaBaseSQLProvider {} - static class SchemaMetaH2Provider extends SchemaMetaBaseSQLProvider {} + static class SchemaMetaH2Provider extends SchemaMetaBaseSQLProvider { + @Override + public String selectSchemaMetaByIdForShare(Long schemaId) { + // H2 has no shared row-lock syntax, so use an exclusive lock in tests. + return selectSchemaMetaByIdForUpdate(schemaId); + } + } public static String listSchemaPOsByFullQualifiedName( @Param("metalakeName") String metalakeName, @Param("catalogName") String catalogName) { @@ -72,6 +78,11 @@ public static String listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId return getProvider().listSchemaPOsByCatalogId(catalogId); } + /** Returns SQL that lists all active schemas in a metalake. */ + public static String listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return getProvider().listSchemaPOsByMetalakeId(metalakeId); + } + public static String selectSchemaIdByCatalogIdAndName( @Param("catalogId") Long catalogId, @Param("schemaName") String name) { return getProvider().selectSchemaIdByCatalogIdAndName(catalogId, name); @@ -93,6 +104,16 @@ public static String selectSchemaMetaById(@Param("schemaId") Long schemaId) { return getProvider().selectSchemaMetaById(schemaId); } + /** Returns SQL that selects and locks an active schema by ID. */ + public static String selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId) { + return getProvider().selectSchemaMetaByIdForUpdate(schemaId); + } + + /** Returns SQL that selects and share-locks an active schema by ID. */ + public static String selectSchemaMetaByIdForShare(@Param("schemaId") Long schemaId) { + return getProvider().selectSchemaMetaByIdForShare(schemaId); + } + public static String insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO) { return getProvider().insertSchemaMeta(schemaPO); } @@ -120,12 +141,15 @@ public static String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List schemaPOs) { + return getProvider().softDeleteSchemaMetasWithVersion(schemaPOs); } public static String deleteSchemaMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java index be03900dc22..43c213fd454 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/CatalogMetaBaseSQLProvider.java @@ -53,6 +53,11 @@ public String listCatalogPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; } + /** Returns SQL that lists and locks all active catalogs in a metalake. */ + public String listCatalogPOsByMetalakeIdForUpdate(@Param("metalakeId") Long metalakeId) { + return listCatalogPOsByMetalakeId(metalakeId) + " FOR UPDATE"; + } + public String listCatalogPOsByCatalogIds(@Param("catalogIds") List catalogIds) { return ""; } public String deleteCatalogMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java index 2524eda76fc..f301c90f1a7 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/MetalakeMetaBaseSQLProvider.java @@ -59,6 +59,16 @@ public String selectMetalakeMetaById(@Param("metalakeId") Long metalakeId) { + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; } + /** Returns SQL that selects and locks an active metalake by ID. */ + public String selectMetalakeMetaByIdForUpdate(@Param("metalakeId") Long metalakeId) { + return selectMetalakeMetaById(metalakeId) + " FOR UPDATE"; + } + + /** Returns SQL that selects and share-locks an active metalake by ID. */ + public String selectMetalakeMetaByIdForShare(@Param("metalakeId") Long metalakeId) { + return selectMetalakeMetaById(metalakeId) + " LOCK IN SHARE MODE"; + } + public String selectMetalakeIdMetaByName(@Param("metalakeName") String metalakeName) { return "SELECT metalake_id as metalakeId" + " FROM " @@ -143,23 +153,18 @@ public String updateMetalakeMeta( + " current_version = #{newMetalakeMeta.currentVersion}," + " last_version = #{newMetalakeMeta.lastVersion}" + " WHERE metalake_id = #{oldMetalakeMeta.metalakeId}" - + " AND metalake_name = #{oldMetalakeMeta.metalakeName}" - + " AND (metalake_comment = #{oldMetalakeMeta.metalakeComment} " - + " OR (metalake_comment IS NULL and #{oldMetalakeMeta.metalakeComment} IS NULL))" - + " AND properties = #{oldMetalakeMeta.properties}" - + " AND audit_info = #{oldMetalakeMeta.auditInfo}" - + " AND schema_version = #{oldMetalakeMeta.schemaVersion}" + " AND current_version = #{oldMetalakeMeta.currentVersion}" - + " AND last_version = #{oldMetalakeMeta.lastVersion}" + " AND deleted_at = 0"; } - public String softDeleteMetalakeMetaByMetalakeId(@Param("metalakeId") Long metalakeId) { + public String softDeleteMetalakeMetaByMetalakeId( + @Param("metalakeId") Long metalakeId, @Param("currentVersion") Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + + " WHERE metalake_id = #{metalakeId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } public String deleteMetalakeMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java index 822ac3cf259..de5d1cdc585 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/base/SchemaMetaBaseSQLProvider.java @@ -38,6 +38,18 @@ public String listSchemaPOsByCatalogId(@Param("catalogId") Long catalogId) { + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; } + /** Returns SQL that lists all active schemas in a metalake. */ + public String listSchemaPOsByMetalakeId(@Param("metalakeId") Long metalakeId) { + return "SELECT schema_id as schemaId, schema_name as schemaName," + + " metalake_id as metalakeId, catalog_id as catalogId," + + " schema_comment as schemaComment, properties, audit_info as auditInfo," + + " current_version as currentVersion, last_version as lastVersion," + + " deleted_at as deletedAt" + + " FROM " + + TABLE_NAME + + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + } + public String listSchemaPOsByFullQualifiedName( @Param("metalakeName") String metalakeName, @Param("catalogName") String catalogName) { return """ @@ -170,6 +182,16 @@ public String selectSchemaMetaById(@Param("schemaId") Long schemaId) { + " WHERE schema_id = #{schemaId} AND deleted_at = 0"; } + /** Returns SQL that selects and locks an active schema by ID. */ + public String selectSchemaMetaByIdForUpdate(@Param("schemaId") Long schemaId) { + return selectSchemaMetaById(schemaId) + " FOR UPDATE"; + } + + /** Returns SQL that selects and share-locks an active schema by ID. */ + public String selectSchemaMetaByIdForShare(@Param("schemaId") Long schemaId) { + return selectSchemaMetaById(schemaId) + " LOCK IN SHARE MODE"; + } + public String insertSchemaMeta(@Param("schemaMeta") SchemaPO schemaPO) { return "INSERT INTO " + TABLE_NAME @@ -273,15 +295,7 @@ public String updateSchemaMeta( + " last_version = #{newSchemaMeta.lastVersion}," + " deleted_at = #{newSchemaMeta.deletedAt}" + " WHERE schema_id = #{oldSchemaMeta.schemaId}" - + " AND schema_name = #{oldSchemaMeta.schemaName}" - + " AND metalake_id = #{oldSchemaMeta.metalakeId}" - + " AND catalog_id = #{oldSchemaMeta.catalogId}" - + " AND (schema_comment = #{oldSchemaMeta.schemaComment}" - + " OR (schema_comment IS NULL and #{oldSchemaMeta.schemaComment} IS NULL))" - + " AND properties = #{oldSchemaMeta.properties}" - + " AND audit_info = #{oldSchemaMeta.auditInfo}" + " AND current_version = #{oldSchemaMeta.currentVersion}" - + " AND last_version = #{oldSchemaMeta.lastVersion}" + " AND deleted_at = 0"; } @@ -299,20 +313,28 @@ public String softDeleteSchemaMetasBySchemaIds(@Param("schemaIds") List sc + ""; } - public String softDeleteSchemaMetasByMetalakeId(@Param("metalakeId") Long metalakeId) { + public String softDeleteSchemaMetaBySchemaIdAndVersion( + @Param("schemaId") Long schemaId, @Param("currentVersion") Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = (UNIX_TIMESTAMP() * 1000.0)" + " + EXTRACT(MICROSECOND FROM CURRENT_TIMESTAMP(3)) / 1000" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + + " WHERE schema_id = #{schemaId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } - public String softDeleteSchemaMetasByCatalogId(@Param("catalogId") Long catalogId) { - return "UPDATE " + /** Returns SQL that soft-deletes schemas using identifier-and-version pairs. */ + public String softDeleteSchemaMetasWithVersion(@Param("schemaMetas") List schemaPOs) { + return ""; } public String deleteSchemaMetasByLegacyTimeline( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java index 0482d9b330b..6f85de6d190 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/CatalogMetaPostgreSQLProvider.java @@ -20,25 +20,38 @@ import static org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper.TABLE_NAME; +import java.util.List; import org.apache.gravitino.storage.relational.mapper.provider.base.CatalogMetaBaseSQLProvider; import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.ibatis.annotations.Param; public class CatalogMetaPostgreSQLProvider extends CatalogMetaBaseSQLProvider { @Override - public String softDeleteCatalogMetasByCatalogId(Long catalogId) { + public String selectCatalogMetaByIdForShare(Long catalogId) { + return selectCatalogMetaById(catalogId) + " FOR SHARE"; + } + + @Override + public String softDeleteCatalogMetasByCatalogId(Long catalogId, Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE catalog_id = #{catalogId} AND deleted_at = 0"; + + " WHERE catalog_id = #{catalogId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } + /** {@inheritDoc} */ @Override - public String softDeleteCatalogMetasByMetalakeId(Long metalakeId) { - return "UPDATE " + public String softDeleteCatalogMetasWithVersion(List catalogPOs) { + return ""; } @Override @@ -101,17 +114,7 @@ public String updateCatalogMeta( + " last_version = #{newCatalogMeta.lastVersion}," + " deleted_at = #{newCatalogMeta.deletedAt}" + " WHERE catalog_id = #{oldCatalogMeta.catalogId}" - + " AND catalog_name = #{oldCatalogMeta.catalogName}" - + " AND metalake_id = #{oldCatalogMeta.metalakeId}" - + " AND type = #{oldCatalogMeta.type}" - + " AND provider = #{oldCatalogMeta.provider}" - + " AND (catalog_comment = #{oldCatalogMeta.catalogComment} " - + " OR (CAST(catalog_comment AS VARCHAR) IS NULL AND " - + " CAST(#{oldCatalogMeta.catalogComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldCatalogMeta.properties}" - + " AND audit_info = #{oldCatalogMeta.auditInfo}" + " AND current_version = #{oldCatalogMeta.currentVersion}" - + " AND last_version = #{oldCatalogMeta.lastVersion}" + " AND deleted_at = 0"; } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java index 5ce01e67159..f0e0db93e11 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/MetalakeMetaPostgreSQLProvider.java @@ -26,11 +26,17 @@ public class MetalakeMetaPostgreSQLProvider extends MetalakeMetaBaseSQLProvider { @Override - public String softDeleteMetalakeMetaByMetalakeId(Long metalakeId) { + public String selectMetalakeMetaByIdForShare(Long metalakeId) { + return selectMetalakeMetaById(metalakeId) + " FOR SHARE"; + } + + @Override + public String softDeleteMetalakeMetaByMetalakeId(Long metalakeId, Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + + " WHERE metalake_id = #{metalakeId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } @Override @@ -75,15 +81,7 @@ public String updateMetalakeMeta( + " current_version = #{newMetalakeMeta.currentVersion}," + " last_version = #{newMetalakeMeta.lastVersion}" + " WHERE metalake_id = #{oldMetalakeMeta.metalakeId}" - + " AND metalake_name = #{oldMetalakeMeta.metalakeName}" - + " AND (metalake_comment = #{oldMetalakeMeta.metalakeComment} " - + " OR (CAST(metalake_comment AS VARCHAR) IS NULL AND " - + " CAST(#{oldMetalakeMeta.metalakeComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldMetalakeMeta.properties}" - + " AND audit_info = #{oldMetalakeMeta.auditInfo}" - + " AND schema_version = #{oldMetalakeMeta.schemaVersion}" + " AND current_version = #{oldMetalakeMeta.currentVersion}" - + " AND last_version = #{oldMetalakeMeta.lastVersion}" + " AND deleted_at = 0"; } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java index ba2087aa61a..8440bf49a62 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/mapper/provider/postgresql/SchemaMetaPostgreSQLProvider.java @@ -26,6 +26,11 @@ import org.apache.ibatis.annotations.Param; public class SchemaMetaPostgreSQLProvider extends SchemaMetaBaseSQLProvider { + @Override + public String selectSchemaMetaByIdForShare(Long schemaId) { + return selectSchemaMetaById(schemaId) + " FOR SHARE"; + } + @Override public String insertSchemaMetaOnDuplicateKeyUpdate(SchemaPO schemaPO) { return "INSERT INTO " @@ -98,16 +103,7 @@ public String updateSchemaMeta( + " last_version = #{newSchemaMeta.lastVersion}," + " deleted_at = #{newSchemaMeta.deletedAt}" + " WHERE schema_id = #{oldSchemaMeta.schemaId}" - + " AND schema_name = #{oldSchemaMeta.schemaName}" - + " AND metalake_id = #{oldSchemaMeta.metalakeId}" - + " AND catalog_id = #{oldSchemaMeta.catalogId}" - + " AND (schema_comment = #{oldSchemaMeta.schemaComment}" - + " OR (CAST(schema_comment AS VARCHAR) IS NULL" - + " AND CAST(#{oldSchemaMeta.schemaComment} AS VARCHAR) IS NULL))" - + " AND properties = #{oldSchemaMeta.properties}" - + " AND audit_info = #{oldSchemaMeta.auditInfo}" + " AND current_version = #{oldSchemaMeta.currentVersion}" - + " AND last_version = #{oldSchemaMeta.lastVersion}" + " AND deleted_at = 0"; } @@ -126,19 +122,26 @@ public String softDeleteSchemaMetasBySchemaIds(List schemaIds) { } @Override - public String softDeleteSchemaMetasByMetalakeId(Long metalakeId) { + public String softDeleteSchemaMetaBySchemaIdAndVersion(Long schemaId, Long currentVersion) { return "UPDATE " + TABLE_NAME + " SET deleted_at = CAST(EXTRACT(EPOCH FROM CURRENT_TIMESTAMP) * 1000 AS BIGINT)" - + " WHERE metalake_id = #{metalakeId} AND deleted_at = 0"; + + " WHERE schema_id = #{schemaId}" + + " AND current_version = #{currentVersion} AND deleted_at = 0"; } + /** {@inheritDoc} */ @Override - public String softDeleteSchemaMetasByCatalogId(Long catalogId) { - return "UPDATE " + public String softDeleteSchemaMetasWithVersion(List schemaPOs) { + return ""; } @Override diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java index 9f63b9189f3..e58fe7fe620 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/CatalogMetaService.java @@ -24,7 +24,6 @@ import java.io.IOException; import java.util.List; import java.util.Objects; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import org.apache.gravitino.Entity; @@ -35,7 +34,6 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.meta.CatalogEntity; -import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.relational.helper.CatalogIds; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; @@ -43,6 +41,7 @@ import org.apache.gravitino.storage.relational.mapper.FilesetVersionMapper; import org.apache.gravitino.storage.relational.mapper.FunctionMetaMapper; import org.apache.gravitino.storage.relational.mapper.FunctionVersionMetaMapper; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; import org.apache.gravitino.storage.relational.mapper.ModelMetaMapper; import org.apache.gravitino.storage.relational.mapper.ModelVersionAliasRelMapper; import org.apache.gravitino.storage.relational.mapper.ModelVersionMetaMapper; @@ -57,6 +56,8 @@ import org.apache.gravitino.storage.relational.mapper.TopicMetaMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; @@ -179,20 +180,32 @@ public void insertCatalog(CatalogEntity catalogEntity, boolean overwrite) throws try { NameIdentifierUtil.checkCatalog(catalogEntity.nameIdentifier()); - String metalake = NameIdentifierUtil.getMetalake(catalogEntity.nameIdentifier()); - Long metalakeId = - EntityIdService.getEntityId(NameIdentifier.of(metalake), Entity.EntityType.METALAKE); - - SessionUtils.doWithCommit( - CatalogMetaMapper.class, - mapper -> { - CatalogPO po = POConverters.initializeCatalogPOWithVersion(catalogEntity, metalakeId); - if (overwrite) { - mapper.insertCatalogMetaOnDuplicateKeyUpdate(po); - } else { - mapper.insertCatalogMeta(po); - } - }); + String metalakeName = NameIdentifierUtil.getMetalake(catalogEntity.nameIdentifier()); + MetalakePO metalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + if (metalakePO == null) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + metalakeName); + } + + SessionUtils.doMultipleWithCommit( + () -> lockMetalakeForCatalogCreate(metalakePO), + () -> + SessionUtils.doWithoutCommit( + CatalogMetaMapper.class, + mapper -> { + CatalogPO po = + POConverters.initializeCatalogPOWithVersion( + catalogEntity, metalakePO.getMetalakeId()); + if (overwrite) { + mapper.insertCatalogMetaOnDuplicateKeyUpdate(po); + } else { + mapper.insertCatalogMeta(po); + } + })); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.CATALOG, catalogEntity.nameIdentifier().toString()); @@ -220,29 +233,28 @@ public CatalogEntity updateCatalog( newEntity.id(), oldCatalogEntity.id()); - AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - CatalogMetaMapper.class, - mapper -> - mapper.updateCatalogMeta( - POConverters.updateCatalogPOWithVersion( - oldCatalogPO, newEntity, oldCatalogPO.getMetalakeId()), - oldCatalogPO)))); + () -> { + int updated = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> + mapper.updateCatalogMeta( + POConverters.updateCatalogPOWithVersion( + oldCatalogPO, newEntity, oldCatalogPO.getMetalakeId()), + oldCatalogPO)); + if (updated == 0) { + throw catalogWriteFailure(identifier, oldCatalogPO); + } + }); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.CATALOG, newEntity.nameIdentifier().toString()); throw re; } - if (updateResult.get() > 0) { - return newEntity; - } else { - throw new IOException("Failed to update the entity: " + identifier); - } + return newEntity; } @Monitored( @@ -252,18 +264,15 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { NameIdentifierUtil.checkCatalog(identifier); String catalogName = identifier.name(); - long catalogId = EntityIdService.getEntityId(identifier, Entity.EntityType.CATALOG); + CatalogPO catalogPO = getCatalogPOByName(identifier.namespace().level(0), catalogName); + long catalogId = catalogPO.getCatalogId(); if (cascade) { SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByCatalogId(catalogId)), - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasByCatalogId(catalogId)), + () -> { + deleteCatalogWithVersion(identifier, catalogPO); + deleteSchemasWithVersions(identifier, catalogId); + }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -328,19 +337,17 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { ViewMetaMapper.class, mapper -> mapper.softDeleteViewMetasByCatalogId(catalogId))); } else { - List schemaEntities = - SchemaMetaService.getInstance() - .listSchemasByNamespace( - NamespaceUtil.ofSchema(identifier.namespace().level(0), catalogName)); - if (!schemaEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByCatalogId(catalogId)), + () -> { + deleteCatalogWithVersion(identifier, catalogPO); + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByCatalogId(catalogId)); + if (!schemaPOs.isEmpty()) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", identifier); + } + }, () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -374,6 +381,66 @@ public boolean deleteCatalog(NameIdentifier identifier, boolean cascade) { return true; } + private void deleteCatalogWithVersion(NameIdentifier identifier, CatalogPO observedCatalogPO) { + int deleted = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId( + observedCatalogPO.getCatalogId(), observedCatalogPO.getCurrentVersion())); + if (deleted == 0) { + throw catalogWriteFailure(identifier, observedCatalogPO); + } + } + + private void lockMetalakeForCatalogCreate(MetalakePO observedMetalakePO) { + MetalakePO currentMetalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.selectMetalakeMetaByIdForShare(observedMetalakePO.getMetalakeId())); + if (currentMetalakePO == null + || !Objects.equals( + currentMetalakePO.getMetalakeName(), observedMetalakePO.getMetalakeName())) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + observedMetalakePO.getMetalakeName()); + } + } + + private RuntimeException catalogWriteFailure( + NameIdentifier identifier, CatalogPO observedCatalogPO) { + CatalogPO currentCatalogPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.selectCatalogMetaByIdForUpdate(observedCatalogPO.getCatalogId())); + if (currentCatalogPO == null + || !Objects.equals(currentCatalogPO.getCatalogName(), observedCatalogPO.getCatalogName()) + || !Objects.equals(currentCatalogPO.getMetalakeId(), observedCatalogPO.getMetalakeId())) { + return new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.CATALOG.name().toLowerCase(), + identifier.name()); + } + return ExceptionUtils.concurrentModification(Entity.EntityType.CATALOG, identifier); + } + + private void deleteSchemasWithVersions(NameIdentifier catalogIdentifier, Long catalogId) { + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByCatalogId(catalogId)); + if (schemaPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(schemaPOs)); + if (deleted != schemaPOs.size()) { + throw ExceptionUtils.concurrentChildModification( + Entity.EntityType.SCHEMA, Entity.EntityType.CATALOG, catalogIdentifier); + } + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteCatalogMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java index 4dbcadbe383..422d1e48a83 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/FilesetMetaService.java @@ -166,6 +166,13 @@ public void insertFileset(FilesetEntity filesetEntity, boolean overwrite) throws // insert both fileset meta table and version table SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + filesetEntity.nameIdentifier(), + po.getSchemaId(), + po.getCatalogId(), + po.getMetalakeId()), () -> SessionUtils.doWithoutCommit( FilesetMetaMapper.class, diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java index 2c582dc8c0d..21ff02b449b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java @@ -115,6 +115,13 @@ public void insertFunction(FunctionEntity functionEntity, boolean overwrite) thr FunctionPO po = initializeFunctionPO(functionEntity, builder); SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + functionEntity.nameIdentifier(), + po.schemaId(), + po.catalogId(), + po.metalakeId()), () -> SessionUtils.doWithoutCommit( FunctionMetaMapper.class, mapper -> ops.insertPO(mapper, po, overwrite)), diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java index ba810a61304..422b9066156 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/MetalakeMetaService.java @@ -25,7 +25,6 @@ import java.io.IOException; import java.util.List; import java.util.Objects; -import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import org.apache.gravitino.Entity; @@ -34,7 +33,6 @@ import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; import org.apache.gravitino.meta.BaseMetalake; -import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; import org.apache.gravitino.storage.relational.mapper.FilesetMetaMapper; @@ -64,12 +62,13 @@ import org.apache.gravitino.storage.relational.mapper.UserMetaMapper; import org.apache.gravitino.storage.relational.mapper.UserRoleRelMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; -import org.apache.gravitino.utils.NamespaceUtil; /** * The service class for metalake metadata. It provides the basic database operations for metalake. @@ -175,25 +174,25 @@ public BaseMetalake updateMetalake( MetalakePO newMetalakePO = POConverters.updateMetalakePOWithVersion(oldMetalakePO, newMetalakeEntity); - AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO)))); + () -> { + int updated = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.updateMetalakeMeta(newMetalakePO, oldMetalakePO)); + if (updated == 0) { + throw metalakeWriteFailure( + ident, oldMetalakePO.getMetalakeId(), oldMetalakePO.getMetalakeName()); + } + }); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.METALAKE, newMetalakeEntity.nameIdentifier().toString()); throw re; } - if (updateResult.get() > 0) { - return newMetalakeEntity; - } else { - throw new IOException("Failed to update the entity: " + ident); - } + return newMetalakeEntity; } @Monitored( @@ -201,22 +200,25 @@ public BaseMetalake updateMetalake( baseMetricName = "deleteMetalake") public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { NameIdentifierUtil.checkMetalake(ident); - Long metalakeId = getMetalakeIdByName(ident.name()); + MetalakePO metalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(ident.name())); + if (metalakePO == null) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + ident.toString()); + } + Long metalakeId = metalakePO.getMetalakeId(); + Long currentVersion = metalakePO.getCurrentVersion(); if (metalakeId != null) { if (cascade) { SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId)), - () -> - SessionUtils.doWithoutCommit( - CatalogMetaMapper.class, - mapper -> mapper.softDeleteCatalogMetasByMetalakeId(metalakeId)), - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasByMetalakeId(metalakeId)), + () -> { + deleteMetalakeWithVersion(ident, metalakeId, currentVersion); + deleteCatalogsWithVersions(ident, metalakeId); + deleteSchemasWithVersions(ident, listSchemaPOsForCascade(metalakeId)); + }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -318,18 +320,18 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { ViewMetaMapper.class, mapper -> mapper.softDeleteViewMetasByMetalakeId(metalakeId))); } else { - List catalogEntities = - CatalogMetaService.getInstance() - .listCatalogsByNamespace(NamespaceUtil.ofCatalog(ident.name())); - if (!catalogEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", ident); - } SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - MetalakeMetaMapper.class, - mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId)), + () -> { + deleteMetalakeWithVersion(ident, metalakeId, currentVersion); + List catalogPOs = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.listCatalogPOsByMetalakeId(metalakeId)); + if (!catalogPOs.isEmpty()) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", ident); + } + }, () -> SessionUtils.doWithoutCommit( UserRoleRelMapper.class, @@ -383,6 +385,68 @@ public boolean deleteMetalake(NameIdentifier ident, boolean cascade) { return true; } + void deleteMetalakeWithVersion(NameIdentifier identifier, Long metalakeId, Long currentVersion) { + int deleted = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.softDeleteMetalakeMetaByMetalakeId(metalakeId, currentVersion)); + if (deleted == 0) { + throw metalakeWriteFailure(identifier, metalakeId, identifier.name()); + } + } + + private RuntimeException metalakeWriteFailure( + NameIdentifier identifier, Long metalakeId, String observedName) { + MetalakePO currentMetalakePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByIdForUpdate(metalakeId)); + if (currentMetalakePO == null + || !Objects.equals(currentMetalakePO.getMetalakeName(), observedName)) { + return new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.METALAKE.name().toLowerCase(), + identifier.name()); + } + return ExceptionUtils.concurrentModification(Entity.EntityType.METALAKE, identifier); + } + + private void deleteCatalogsWithVersions(NameIdentifier metalakeIdentifier, Long metalakeId) { + List catalogPOs = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.listCatalogPOsByMetalakeIdForUpdate(metalakeId)); + if (catalogPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.softDeleteCatalogMetasWithVersion(catalogPOs)); + if (deleted != catalogPOs.size()) { + throw ExceptionUtils.concurrentChildModification( + Entity.EntityType.CATALOG, Entity.EntityType.METALAKE, metalakeIdentifier); + } + } + + List listSchemaPOsForCascade(Long metalakeId) { + return SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.listSchemaPOsByMetalakeId(metalakeId)); + } + + private void deleteSchemasWithVersions( + NameIdentifier metalakeIdentifier, List schemaPOs) { + if (schemaPOs.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(schemaPOs)); + if (deleted != schemaPOs.size()) { + throw ExceptionUtils.concurrentChildModification( + Entity.EntityType.SCHEMA, Entity.EntityType.METALAKE, metalakeIdentifier); + } + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteMetalakeMetasByLegacyTimeline") diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java index 838a17fdefc..960c38f435c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/ModelMetaService.java @@ -96,17 +96,26 @@ public void insertModel(ModelEntity modelEntity, boolean overwrite) throws IOExc try { ModelPO.Builder builder = ModelPO.builder(); fillModelPOBuilderParentEntityId(builder, modelEntity.namespace()); + ModelPO po = POConverters.initializeModelPO(modelEntity, builder); - SessionUtils.doWithCommit( - ModelMetaMapper.class, - mapper -> { - ModelPO po = POConverters.initializeModelPO(modelEntity, builder); - if (overwrite) { - mapper.insertModelMetaOnDuplicateKeyUpdate(po); - } else { - mapper.insertModelMeta(po); - } - }); + SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + modelEntity.nameIdentifier(), + po.getSchemaId(), + po.getCatalogId(), + po.getMetalakeId()), + () -> + SessionUtils.doWithoutCommit( + ModelMetaMapper.class, + mapper -> { + if (overwrite) { + mapper.insertModelMetaOnDuplicateKeyUpdate(po); + } else { + mapper.insertModelMeta(po); + } + })); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.MODEL, modelEntity.nameIdentifier().toString()); diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java index 898687f80f8..a3431acd18b 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/SchemaMetaService.java @@ -26,9 +26,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; -import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -40,15 +40,11 @@ import org.apache.gravitino.Namespace; import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; -import org.apache.gravitino.meta.FilesetEntity; -import org.apache.gravitino.meta.ModelEntity; -import org.apache.gravitino.meta.NamespacedEntityId; import org.apache.gravitino.meta.SchemaEntity; -import org.apache.gravitino.meta.TableEntity; -import org.apache.gravitino.meta.TopicEntity; import org.apache.gravitino.metrics.Monitored; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.relational.helper.SchemaIds; +import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; import org.apache.gravitino.storage.relational.mapper.FilesetMetaMapper; import org.apache.gravitino.storage.relational.mapper.FilesetVersionMapper; import org.apache.gravitino.storage.relational.mapper.FunctionMetaMapper; @@ -66,6 +62,7 @@ import org.apache.gravitino.storage.relational.mapper.TagMetadataObjectRelMapper; import org.apache.gravitino.storage.relational.mapper.TopicMetaMapper; import org.apache.gravitino.storage.relational.mapper.ViewMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.utils.ExceptionUtils; import org.apache.gravitino.storage.relational.utils.POConverters; @@ -143,6 +140,10 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) throws IO // rewriter to translate each PO's name to storage form before SQL execution. String logicalSep = HierarchicalSchemaUtil.schemaSeparator(); String schemaName = schemaEntity.name(); + String metalakeName = schemaEntity.namespace().level(0); + String catalogName = schemaEntity.namespace().level(1); + CatalogPO catalogPO = + CatalogMetaService.getInstance().getCatalogPOByName(metalakeName, catalogName); List rowsToInsert = new ArrayList<>(); if (schemaName == null || !schemaName.contains(logicalSep)) { rowsToInsert.add(schemaEntity); @@ -165,39 +166,44 @@ public void insertSchema(SchemaEntity schemaEntity, boolean overwrite) throws IO rowsToInsert.add(schemaEntity); } - SessionUtils.doWithCommit( - SchemaMetaMapper.class, - mapper -> { - int n = rowsToInsert.size(); - List missingAncestorPOs = new ArrayList<>(); - if (n > 1) { - SchemaEntity firstAncestor = rowsToInsert.get(0); - Namespace ancestorNs = firstAncestor.namespace(); - List ancestorNames = - rowsToInsert.subList(0, n - 1).stream() - .map(SchemaEntity::name) - .collect(Collectors.toList()); - Set existingLogicalNames = - ops.listPOs(mapper, ancestorNs, ancestorNames).stream() - .map(SchemaPO::getSchemaName) - .collect(Collectors.toSet()); - for (SchemaEntity row : rowsToInsert.subList(0, n - 1)) { - if (existingLogicalNames.contains(row.name())) { - continue; - } - SchemaPO.Builder builder = SchemaPO.builder(); - fillSchemaPOBuilderParentEntityId(builder, row.namespace()); - missingAncestorPOs.add(POConverters.initializeSchemaPOWithVersion(row, builder)); - } - } - SchemaEntity leafRow = rowsToInsert.get(n - 1); - SchemaPO.Builder leafBuilder = SchemaPO.builder(); - fillSchemaPOBuilderParentEntityId(leafBuilder, leafRow.namespace()); - SchemaPO leafPO = POConverters.initializeSchemaPOWithVersion(leafRow, leafBuilder); - List schemaPosToInsert = new ArrayList<>(missingAncestorPOs); - schemaPosToInsert.add(leafPO); - ops.batchInsertPOs(mapper, schemaPosToInsert, overwrite); - }); + SessionUtils.doMultipleWithCommit( + () -> lockCatalogForSchemaCreate(catalogPO, rowsToInsert.size() > 1), + () -> + SessionUtils.doWithoutCommit( + SchemaMetaMapper.class, + mapper -> { + int n = rowsToInsert.size(); + List missingAncestorPOs = new ArrayList<>(); + if (n > 1) { + SchemaEntity firstAncestor = rowsToInsert.get(0); + Namespace ancestorNs = firstAncestor.namespace(); + List ancestorNames = + rowsToInsert.subList(0, n - 1).stream() + .map(SchemaEntity::name) + .collect(Collectors.toList()); + Map existingAncestors = + ops.listPOs(mapper, ancestorNs, ancestorNames).stream() + .collect( + Collectors.toMap(SchemaPO::getSchemaName, Function.identity())); + for (SchemaEntity row : rowsToInsert.subList(0, n - 1)) { + SchemaPO existingAncestor = existingAncestors.get(row.name()); + if (existingAncestor != null) { + continue; + } + SchemaPO.Builder builder = newSchemaPOBuilder(catalogPO); + missingAncestorPOs.add( + POConverters.initializeSchemaPOWithVersion(row, builder)); + } + } + if (!missingAncestorPOs.isEmpty()) { + ops.batchInsertPOs(mapper, missingAncestorPOs, false); + } + SchemaEntity leafRow = rowsToInsert.get(n - 1); + SchemaPO leafPO = + POConverters.initializeSchemaPOWithVersion( + leafRow, newSchemaPOBuilder(catalogPO)); + ops.batchInsertPOs(mapper, Collections.singletonList(leafPO), overwrite); + })); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.SCHEMA, schemaEntity.nameIdentifier().toString()); @@ -219,29 +225,28 @@ public SchemaEntity updateSchema( newEntity.id(), oldSchemaEntity.id()); - AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( - () -> - updateResult.set( - SessionUtils.getWithoutCommit( - SchemaMetaMapper.class, - mapper -> - ops.updatePO( - mapper, - POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), - oldSchemaPO)))); + () -> { + int updated = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + ops.updatePO( + mapper, + POConverters.updateSchemaPOWithVersion(oldSchemaPO, newEntity), + oldSchemaPO)); + if (updated == 0) { + throw schemaWriteFailure(identifier, oldSchemaPO); + } + }); } catch (RuntimeException re) { ExceptionUtils.checkSQLException( re, Entity.EntityType.SCHEMA, newEntity.nameIdentifier().toString()); throw re; } - if (updateResult.get() > 0) { - return newEntity; - } else { - throw new IOException("Failed to update the entity: " + identifier); - } + return newEntity; } @Monitored( @@ -250,140 +255,93 @@ public SchemaEntity updateSchema( public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { NameIdentifierUtil.checkSchema(identifier); - String schemaName = identifier.name(); SchemaPO schemaPO = getSchemaPOByIdentifier(identifier); Long schemaId = schemaPO.getSchemaId(); if (cascade) { - // For HierarchicalSchema, deleting `A:B` must also cascade into all descendant schemas - // such as `A:B:C`, `A:B:C:D`, etc. Collect the descendant schema ids up-front and run a - // single batch UPDATE per child table so the total SQL cost stays bounded regardless of - // how many descendants exist. - List schemaIds = listSchemaIdsForCascade(schemaPO); - if (schemaIds.isEmpty()) { - return false; - } + AtomicReference> schemaIds = new AtomicReference<>(); SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasBySchemaIds(schemaIds)), + () -> { + lockCatalogForSchemaDelete(identifier, schemaPO); + deleteSchemaWithVersion(identifier, schemaPO); + List descendants = listDescendantSchemaPOs(schemaPO); + deleteDescendantSchemasWithVersions(identifier, descendants); + List ids = new ArrayList<>(descendants.size() + 1); + ids.add(schemaId); + descendants.stream().map(SchemaPO::getSchemaId).forEach(ids::add); + schemaIds.set(ids); + }, () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, - mapper -> mapper.softDeleteTableMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTableMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TableColumnMapper.class, - mapper -> mapper.softDeleteColumnsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteColumnsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FilesetMetaMapper.class, - mapper -> mapper.softDeleteFilesetMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFilesetMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FilesetVersionMapper.class, - mapper -> mapper.softDeleteFilesetVersionsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFilesetVersionsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TopicMetaMapper.class, - mapper -> mapper.softDeleteTopicMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTopicMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FunctionMetaMapper.class, - mapper -> mapper.softDeleteFunctionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFunctionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( FunctionVersionMetaMapper.class, - mapper -> mapper.softDeleteFunctionVersionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteFunctionVersionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( - OwnerMetaMapper.class, mapper -> mapper.softDeleteOwnerRelBySchemaIds(schemaIds)), + OwnerMetaMapper.class, + mapper -> mapper.softDeleteOwnerRelBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( SecurableObjectMapper.class, - mapper -> mapper.softDeleteObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( TagMetadataObjectRelMapper.class, - mapper -> mapper.softDeleteTagMetadataObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteTagMetadataObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( PolicyMetadataObjectRelMapper.class, - mapper -> mapper.softDeletePolicyMetadataObjectRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeletePolicyMetadataObjectRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelVersionAliasRelMapper.class, - mapper -> mapper.softDeleteModelVersionAliasRelsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelVersionAliasRelsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelVersionMetaMapper.class, - mapper -> mapper.softDeleteModelVersionMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelVersionMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ModelMetaMapper.class, - mapper -> mapper.softDeleteModelMetasBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteModelMetasBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( StatisticMetaMapper.class, - mapper -> mapper.softDeleteStatisticsBySchemaIds(schemaIds)), + mapper -> mapper.softDeleteStatisticsBySchemaIds(schemaIds.get())), () -> SessionUtils.doWithoutCommit( ViewMetaMapper.class, - mapper -> mapper.softDeleteViewMetasBySchemaIds(schemaIds))); + mapper -> mapper.softDeleteViewMetasBySchemaIds(schemaIds.get()))); } else { - List tableEntities = - TableMetaService.getInstance() - .listTablesByNamespace( - NamespaceUtil.ofTable( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!tableEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - List filesetEntities = - FilesetMetaService.getInstance() - .listFilesetsByNamespace( - NamespaceUtil.ofFileset( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!filesetEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - List modelEntities = - ModelMetaService.getInstance() - .listModelsByNamespace( - NamespaceUtil.ofModel( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!modelEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - - List topicEntities = - TopicMetaService.getInstance() - .listTopicsByNamespace( - NamespaceUtil.ofTopic( - identifier.namespace().level(0), - identifier.namespace().level(1), - schemaName)); - if (!topicEntities.isEmpty()) { - throw new NonEmptyEntityException( - "Entity %s has sub-entities, you should remove sub-entities first", identifier); - } - - List singleSchemaId = Collections.singletonList(schemaId); SessionUtils.doMultipleWithCommit( - () -> - SessionUtils.doWithoutCommit( - SchemaMetaMapper.class, - mapper -> mapper.softDeleteSchemaMetasBySchemaIds(singleSchemaId)), + () -> { + lockCatalogForSchemaDelete(identifier, schemaPO); + deleteSchemaWithVersion(identifier, schemaPO); + checkSchemaIsEmpty(identifier, schemaPO); + }, () -> SessionUtils.doWithoutCommit( OwnerMetaMapper.class, @@ -416,6 +374,18 @@ public boolean deleteSchema(NameIdentifier identifier, boolean cascade) { return true; } + private void deleteSchemaWithVersion(NameIdentifier identifier, SchemaPO observedSchemaPO) { + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + observedSchemaPO.getSchemaId(), observedSchemaPO.getCurrentVersion())); + if (deleted == 0) { + throw schemaWriteFailure(identifier, observedSchemaPO); + } + } + @Monitored( metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, baseMetricName = "deleteSchemaMetasByLegacyTimeline") @@ -449,13 +419,150 @@ private List listSchemaPOs(Namespace namespace) { mapper -> POStorageReadRouting.listPOs(mapper, namespace, ops, Entity.EntityType.SCHEMA)); } + private void lockCatalogForSchemaCreate( + CatalogPO observedCatalogPO, boolean createsImplicitAncestors) { + CatalogPO currentCatalogPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> + createsImplicitAncestors + ? mapper.selectCatalogMetaByIdForUpdate(observedCatalogPO.getCatalogId()) + : mapper.selectCatalogMetaByIdForShare(observedCatalogPO.getCatalogId())); + if (currentCatalogPO == null + || !Objects.equals(currentCatalogPO.getCatalogName(), observedCatalogPO.getCatalogName()) + || !Objects.equals(currentCatalogPO.getMetalakeId(), observedCatalogPO.getMetalakeId())) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.CATALOG.name().toLowerCase(), + observedCatalogPO.getCatalogName()); + } + } + + private void lockCatalogForSchemaDelete(NameIdentifier identifier, SchemaPO observedSchemaPO) { + CatalogPO currentCatalogPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.selectCatalogMetaByIdForUpdate(observedSchemaPO.getCatalogId())); + if (currentCatalogPO == null + || !Objects.equals(currentCatalogPO.getCatalogName(), identifier.namespace().level(1)) + || !Objects.equals(currentCatalogPO.getMetalakeId(), observedSchemaPO.getMetalakeId())) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.CATALOG.name().toLowerCase(), + identifier.namespace().level(1)); + } + } + + void lockSchemaForEntityWrite( + NameIdentifier entityIdentifier, + Long observedSchemaId, + Long observedCatalogId, + Long observedMetalakeId) { + NameIdentifier schemaIdentifier = NameIdentifierUtil.getSchemaIdentifier(entityIdentifier); + SchemaPO currentSchemaPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.selectSchemaMetaByIdForShare(observedSchemaId)); + if (currentSchemaPO != null) { + currentSchemaPO = physicalToLogicalSchemaPO(currentSchemaPO); + } + if (currentSchemaPO == null + || !Objects.equals(currentSchemaPO.getSchemaName(), schemaIdentifier.name()) + || !Objects.equals(currentSchemaPO.getCatalogId(), observedCatalogId) + || !Objects.equals(currentSchemaPO.getMetalakeId(), observedMetalakeId)) { + throw noSuchSchemaException(schemaIdentifier); + } + } + + private RuntimeException schemaWriteFailure( + NameIdentifier identifier, SchemaPO observedSchemaPO) { + SchemaPO currentSchemaPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.selectSchemaMetaByIdForUpdate(observedSchemaPO.getSchemaId())); + if (currentSchemaPO == null) { + return noSuchSchemaException(identifier); + } + currentSchemaPO = physicalToLogicalSchemaPO(currentSchemaPO); + if (!Objects.equals(currentSchemaPO.getSchemaName(), observedSchemaPO.getSchemaName()) + || !Objects.equals(currentSchemaPO.getCatalogId(), observedSchemaPO.getCatalogId()) + || !Objects.equals(currentSchemaPO.getMetalakeId(), observedSchemaPO.getMetalakeId())) { + return noSuchSchemaException(identifier); + } + return ExceptionUtils.concurrentModification(Entity.EntityType.SCHEMA, identifier); + } + + private NoSuchEntityException noSuchSchemaException(NameIdentifier identifier) { + return new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.SCHEMA.name().toLowerCase(), + identifier.name()); + } + + private void deleteDescendantSchemasWithVersions( + NameIdentifier schemaIdentifier, List descendants) { + if (descendants.isEmpty()) { + return; + } + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.softDeleteSchemaMetasWithVersion(descendants)); + if (deleted != descendants.size()) { + throw ExceptionUtils.concurrentChildModification( + Entity.EntityType.SCHEMA, Entity.EntityType.SCHEMA, schemaIdentifier); + } + } + + private void checkSchemaIsEmpty(NameIdentifier identifier, SchemaPO schemaPO) { + boolean hasDescendantSchemas = !listDescendantSchemaPOs(schemaPO).isEmpty(); + boolean hasTables = + !SessionUtils.getWithoutCommit( + TableMetaMapper.class, + mapper -> mapper.listTablePOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasFilesets = + !SessionUtils.getWithoutCommit( + FilesetMetaMapper.class, + mapper -> mapper.listFilesetPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasModels = + !SessionUtils.getWithoutCommit( + ModelMetaMapper.class, + mapper -> mapper.listModelPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasTopics = + !SessionUtils.getWithoutCommit( + TopicMetaMapper.class, + mapper -> mapper.listTopicPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasViews = + !SessionUtils.getWithoutCommit( + ViewMetaMapper.class, + mapper -> mapper.listViewPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + boolean hasFunctions = + !SessionUtils.getWithoutCommit( + FunctionMetaMapper.class, + mapper -> mapper.listFunctionPOsBySchemaId(schemaPO.getSchemaId())) + .isEmpty(); + if (hasDescendantSchemas + || hasTables + || hasFilesets + || hasModels + || hasTopics + || hasViews + || hasFunctions) { + throw new NonEmptyEntityException( + "Entity %s has sub-entities, you should remove sub-entities first", identifier); + } + } + /** - * Collects the schema ids that participate in a cascade delete: the target schema itself plus - * every HierarchicalSchema descendant. The {@link SchemaPO} arrives in logical form (e.g. {@code - * A:B}); {@link HierarchicalConversionPOStorageOps} translates to storage form before running the - * SQL prefix match, so this method only deals in logical names. + * Collects every HierarchicalSchema descendant of the target schema. The {@link SchemaPO} arrives + * in logical form (e.g. {@code A:B}); {@link HierarchicalConversionPOStorageOps} translates to + * storage form before running the SQL prefix match. */ - private List listSchemaIdsForCascade(SchemaPO schemaPO) { + private List listDescendantSchemaPOs(SchemaPO schemaPO) { List matched = SessionUtils.getWithoutCommit( SchemaMetaMapper.class, @@ -464,16 +571,15 @@ private List listSchemaIdsForCascade(SchemaPO schemaPO) { if (matched == null || matched.isEmpty()) { return Collections.emptyList(); } - return matched.stream().map(SchemaPO::getSchemaId).collect(Collectors.toList()); + return matched.stream() + .filter(po -> !po.getSchemaId().equals(schemaPO.getSchemaId())) + .collect(Collectors.toList()); } - private void fillSchemaPOBuilderParentEntityId(SchemaPO.Builder builder, Namespace namespace) { - NamespaceUtil.checkSchema(namespace); - NamespacedEntityId namespacedEntityId = - EntityIdService.getEntityIds( - NameIdentifier.of(namespace.levels()), Entity.EntityType.CATALOG); - builder.withMetalakeId(namespacedEntityId.namespaceIds()[0]); - builder.withCatalogId(namespacedEntityId.entityId()); + private SchemaPO.Builder newSchemaPOBuilder(CatalogPO catalogPO) { + return SchemaPO.builder() + .withMetalakeId(catalogPO.getMetalakeId()) + .withCatalogId(catalogPO.getCatalogId()); } @Monitored( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java index 15a61bdd9fd..6e63eb0301c 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/TableMetaService.java @@ -126,6 +126,13 @@ public void insertTable(TableEntity tableEntity, boolean overwrite) throws IOExc AtomicReference tablePORef = new AtomicReference<>(); TablePO po = POConverters.initializeTablePOWithVersion(tableEntity, builder); SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + tableEntity.nameIdentifier(), + po.getSchemaId(), + po.getCatalogId(), + po.getMetalakeId()), () -> SessionUtils.doWithoutCommit( TableMetaMapper.class, @@ -194,6 +201,16 @@ public TableEntity updateTable( final AtomicInteger updateResult = new AtomicInteger(0); try { SessionUtils.doMultipleWithCommit( + () -> { + if (isSchemaChanged) { + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + newTableEntity.nameIdentifier(), + newSchemaId, + oldTablePO.getCatalogId(), + oldTablePO.getMetalakeId()); + } + }, () -> updateResult.set( SessionUtils.getWithoutCommit( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/TopicMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/TopicMetaService.java index d5618842d0e..ef49ad2c984 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/TopicMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/TopicMetaService.java @@ -70,17 +70,26 @@ public void insertTopic(TopicEntity topicEntity, boolean overwrite) throws IOExc TopicPO.Builder builder = TopicPO.builder(); fillTopicPOBuilderParentEntityId(builder, topicEntity.namespace()); + TopicPO po = POConverters.initializeTopicPOWithVersion(topicEntity, builder); - SessionUtils.doWithCommit( - TopicMetaMapper.class, - mapper -> { - TopicPO po = POConverters.initializeTopicPOWithVersion(topicEntity, builder); - if (overwrite) { - mapper.insertTopicMetaOnDuplicateKeyUpdate(po); - } else { - mapper.insertTopicMeta(po); - } - }); + SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + topicEntity.nameIdentifier(), + po.getSchemaId(), + po.getCatalogId(), + po.getMetalakeId()), + () -> + SessionUtils.doWithoutCommit( + TopicMetaMapper.class, + mapper -> { + if (overwrite) { + mapper.insertTopicMetaOnDuplicateKeyUpdate(po); + } else { + mapper.insertTopicMeta(po); + } + })); // TODO: insert topic dataLayout version after supporting it } catch (RuntimeException re) { ExceptionUtils.checkSQLException( diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java b/core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java index d676b6bf08a..5a98bdcf7d8 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/service/ViewMetaService.java @@ -106,6 +106,13 @@ public void insertView(ViewEntity viewEntity, boolean overwrite) throws IOExcept ViewPO po = initializeViewPO(viewEntity, builder); SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + viewEntity.nameIdentifier(), + po.getSchemaId(), + po.getCatalogId(), + po.getMetalakeId()), () -> SessionUtils.doWithoutCommit( ViewMetaMapper.class, mapper -> ops.insertPO(mapper, po, overwrite)), diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/ExceptionUtils.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/ExceptionUtils.java index eb08cfd2e89..7da30085003 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/ExceptionUtils.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/ExceptionUtils.java @@ -20,7 +20,10 @@ import java.io.IOException; import java.sql.SQLException; +import java.util.Locale; import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.storage.relational.converters.SQLExceptionConverterFactory; public class ExceptionUtils { @@ -33,4 +36,37 @@ public static void checkSQLException( .toGravitinoException((SQLException) re.getCause(), type, entityName); } } + + /** + * Creates an {@link OptimisticLockException} for an entity that was modified concurrently by + * another writer, which makes the version-guarded write fail. + * + * @param type The type of the entity that was modified concurrently. + * @param identifier The identifier of the entity that was modified concurrently. + * @return The {@link OptimisticLockException} to throw. + */ + public static OptimisticLockException concurrentModification( + Entity.EntityType type, NameIdentifier identifier) { + return new OptimisticLockException( + "The %s %s was modified concurrently; retry the operation", + type.name().toLowerCase(Locale.ROOT), identifier); + } + + /** + * Creates an {@link OptimisticLockException} for a child entity that was modified concurrently + * while its parent was being deleted in cascade mode. + * + * @param childType The type of the child entity that was modified concurrently. + * @param parentType The type of the parent entity being operated on. + * @param parentIdentifier The identifier of the parent entity being operated on. + * @return The {@link OptimisticLockException} to throw. + */ + public static OptimisticLockException concurrentChildModification( + Entity.EntityType childType, Entity.EntityType parentType, NameIdentifier parentIdentifier) { + return new OptimisticLockException( + "A %s under %s %s was modified concurrently; retry the operation", + childType.name().toLowerCase(Locale.ROOT), + parentType.name().toLowerCase(Locale.ROOT), + parentIdentifier); + } } diff --git a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java index d019800d1ab..57efbd4767d 100644 --- a/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java +++ b/core/src/main/java/org/apache/gravitino/storage/relational/utils/POConverters.java @@ -137,9 +137,9 @@ public static MetalakePO initializeMetalakePOWithVersion(BaseMetalake baseMetala */ public static MetalakePO updateMetalakePOWithVersion( MetalakePO oldMetalakePO, BaseMetalake newMetalake) { - Long lastVersion = oldMetalakePO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // metalakes do not retain independently addressable historical versions. + Long nextVersion = oldMetalakePO.getCurrentVersion() + 1; try { return MetalakePO.builder() .withMetalakeId(newMetalake.id()) @@ -234,9 +234,9 @@ public static CatalogPO initializeCatalogPOWithVersion( */ public static CatalogPO updateCatalogPOWithVersion( CatalogPO oldCatalogPO, CatalogEntity newCatalog, Long metalakeId) { - Long lastVersion = oldCatalogPO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // catalogs do not retain independently addressable historical versions. + Long nextVersion = oldCatalogPO.getCurrentVersion() + 1; try { return CatalogPO.builder() .withCatalogId(newCatalog.id()) @@ -330,9 +330,9 @@ public static SchemaPO initializeSchemaPOWithVersion( * @return SchemaPO object with updated version */ public static SchemaPO updateSchemaPOWithVersion(SchemaPO oldSchemaPO, SchemaEntity newSchema) { - Long lastVersion = oldSchemaPO.getLastVersion(); - // Will set the version to the last version + 1 when having some fields need be multiple version - Long nextVersion = lastVersion; + // Every metadata update advances the OCC token. Both version columns stay aligned because + // schemas do not retain independently addressable historical versions. + Long nextVersion = oldSchemaPO.getCurrentVersion() + 1; try { return SchemaPO.builder() .withSchemaId(oldSchemaPO.getSchemaId()) 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 c55c4044692..36e8f370ceb 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestCatalogManager.java @@ -57,6 +57,7 @@ import org.apache.gravitino.connector.capability.CapabilityResult; import org.apache.gravitino.exceptions.CatalogAlreadyExistsException; import org.apache.gravitino.exceptions.NoSuchCatalogException; +import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.exceptions.NoSuchSchemaException; import org.apache.gravitino.lock.LockManager; @@ -850,6 +851,26 @@ void testDropCatalogDoesNotMarkLocalMutationWhenStoreReturnsFalse() throws Excep manager.close(); } + @Test + void testDropCatalogReturnsFalseWhenConcurrentDeleteWins() throws Exception { + ChangeLogAwareEntityStore store = new ChangeLogAwareEntityStore(); + store.initialize(config); + store.put(metalakeEntity, true); + + CatalogManager manager = + new CatalogManager(config, store, new RandomIdGenerator(), new SecretManager(config)); + NameIdentifier ident = NameIdentifier.of("metalake", "concurrently_deleted"); + Map props = + ImmutableMap.of( + PROPERTY_KEY1, "value1", PROPERTY_KEY2, "value2", PROPERTY_KEY5_PREFIX + "1", "value3"); + manager.createCatalog(ident, Catalog.Type.RELATIONAL, provider, "comment", props); + store.throwMissingForCatalogDelete = true; + + Assertions.assertFalse(manager.dropCatalog(ident, true)); + Assertions.assertNull(manager.getCatalogCache().getIfPresent(ident)); + manager.close(); + } + @Test void testFailedCreateCatalogCleanupMarksLocalMutation() throws Exception { ChangeLogAwareEntityStore store = new ChangeLogAwareEntityStore(); @@ -983,6 +1004,7 @@ private static class ChangeLogAwareEntityStore extends InMemoryEntityStore private final AtomicReference unregisteredListener = new AtomicReference<>(); private boolean returnFalseForCatalogDelete; + private boolean throwMissingForCatalogDelete; @Override public boolean delete(NameIdentifier ident, EntityType entityType, boolean cascade) @@ -990,6 +1012,12 @@ public boolean delete(NameIdentifier ident, EntityType entityType, boolean casca if (returnFalseForCatalogDelete && entityType == EntityType.CATALOG) { return false; } + if (throwMissingForCatalogDelete && entityType == EntityType.CATALOG) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + EntityType.CATALOG.name().toLowerCase(), + ident.toString()); + } return super.delete(ident, entityType, cascade); } diff --git a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java index c5be0774ea8..55084847f33 100644 --- a/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java +++ b/core/src/test/java/org/apache/gravitino/metalake/TestMetalakeManager.java @@ -31,6 +31,7 @@ import java.util.Set; import org.apache.commons.lang3.reflect.FieldUtils; import org.apache.gravitino.Config; +import org.apache.gravitino.Entity.EntityType; import org.apache.gravitino.EntityStore; import org.apache.gravitino.GravitinoEnv; import org.apache.gravitino.MetalakeChange; @@ -39,11 +40,13 @@ import org.apache.gravitino.UserPrincipal; import org.apache.gravitino.auth.AuthConstants; import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException; +import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; import org.apache.gravitino.lock.LockManager; import org.apache.gravitino.meta.BaseMetalake; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.memory.TestMemoryEntityStore; +import org.apache.gravitino.storage.memory.TestMemoryEntityStore.InMemoryEntityStore; import org.apache.gravitino.utils.PrincipalUtils; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; @@ -210,6 +213,23 @@ public void testDropMetalake() { Assertions.assertFalse(dropped1, "metalake should be non-existent"); } + @Test + public void testDropMetalakeReturnsFalseWhenConcurrentDeleteWins() throws IOException { + InMemoryEntityStore store = Mockito.spy(new InMemoryEntityStore()); + store.initialize(config); + MetalakeManager manager = new MetalakeManager(store, new RandomIdGenerator()); + NameIdentifier ident = NameIdentifier.of("concurrently_deleted_metalake"); + manager.createMetalake(ident, "comment", ImmutableMap.of()); + Mockito.doThrow( + new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, "metalake", ident.toString())) + .when(store) + .delete(ident, EntityType.METALAKE, true); + + Assertions.assertFalse(manager.dropMetalake(ident, true)); + store.close(); + } + @Test public void testListInUseMetalakes() { // Create some metalakes with different in-use status diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java index de5520908af..28f19ea19cc 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestCatalogMetaService.java @@ -29,12 +29,22 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Arrays; import java.util.List; +import java.util.Objects; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.gravitino.Catalog; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.AuditInfo; import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.ColumnEntity; @@ -50,7 +60,11 @@ import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.MetalakePO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.gravitino.storage.relational.utils.POConverters; import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; @@ -88,6 +102,84 @@ public void testInsertAlreadyExistsException() throws IOException { assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(catalogCopy, false)); } + @TestTemplate + public void testInsertCatalogLocksMetalakeWithoutChangingVersion() throws IOException { + MetalakePO beforeInsert = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_fence", + auditInfo); + backend.insert(catalog, false); + + MetalakePO afterInsert = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + assertEquals(beforeInsert.getCurrentVersion(), afterInsert.getCurrentVersion()); + assertEquals(beforeInsert.getLastVersion(), afterInsert.getLastVersion()); + + CatalogEntity duplicate = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + catalog.name(), + auditInfo); + assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(duplicate, false)); + + MetalakePO afterFailure = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalakeName)); + assertEquals(afterInsert.getCurrentVersion(), afterFailure.getCurrentVersion()); + assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); + } + + @TestTemplate + public void testConcurrentSameNameCatalogCreateReportsAlreadyExists() throws Exception { + CatalogEntity first = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog", + auditInfo); + CatalogEntity second = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + first.name(), + auditInfo); + + List results = insertCatalogsConcurrently(first, second); + assertEquals(1, results.stream().filter(Objects::isNull).count()); + Throwable failure = results.stream().filter(Objects::nonNull).findFirst().orElseThrow(); + Assertions.assertTrue( + failure instanceof EntityAlreadyExistsException, + () -> "Expected EntityAlreadyExistsException, but got " + failure); + } + + @TestTemplate + public void testConcurrentDifferentCatalogCreatesBothSucceed() throws Exception { + CatalogEntity first = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog_1", + auditInfo); + CatalogEntity second = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "concurrent_catalog_2", + auditInfo); + + List results = insertCatalogsConcurrently(first, second); + Assertions.assertTrue( + results.stream().allMatch(Objects::isNull), + () -> "Concurrent catalog creates failed: " + results); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { CatalogEntity catalog = @@ -149,6 +241,152 @@ void testUpdateCatalogWithNullableComment() throws IOException { Assertions.assertNotNull(updatedCatalog.getComment()); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_occ", + auditInfo); + backend.insert(catalog, false); + CatalogPO oldPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + CatalogEntity updatedCatalog = + CatalogEntity.builder() + .withId(catalog.id()) + .withName(catalog.name()) + .withNamespace(catalog.namespace()) + .withAuditInfo(auditInfo) + .withComment("updated") + .withProperties(catalog.getProperties()) + .withType(catalog.getType()) + .withProvider(catalog.getProvider()) + .build(); + CatalogPO newPO = + POConverters.updateCatalogPOWithVersion(oldPO, updatedCatalog, oldPO.getMetalakeId()); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, mapper -> mapper.updateCatalogMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, mapper -> mapper.updateCatalogMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId(catalog.id(), oldPO.getCurrentVersion())); + assertEquals(1, updated); + assertEquals(0, staleUpdate); + assertEquals(0, staleDelete); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId(catalog.id(), newPO.getCurrentVersion())); + assertEquals(1, deleted); + } + + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_alter_conflict", + auditInfo); + backend.insert(catalog, false); + + assertThrows( + OptimisticLockException.class, + () -> + CatalogMetaService.getInstance() + .updateCatalog( + catalog.nameIdentifier(), + entity -> { + CatalogEntity current = (CatalogEntity) entity; + CatalogPO currentPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.selectCatalogMetaById(current.id())); + CatalogEntity competingUpdate = + copyCatalogWithComment(current, "competing update"); + CatalogPO competingPO = + POConverters.updateCatalogPOWithVersion( + currentPO, competingUpdate, currentPO.getMetalakeId()); + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> mapper.updateCatalogMeta(competingPO, currentPO)); + return copyCatalogWithComment(current, "requested update"); + })); + } + + @TestTemplate + public void testAlterReportsNoSuchWhenCatalogIsDeletedConcurrently() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_alter_deleted", + auditInfo); + backend.insert(catalog, false); + + assertThrows( + NoSuchEntityException.class, + () -> + CatalogMetaService.getInstance() + .updateCatalog( + catalog.nameIdentifier(), + entity -> { + CatalogEntity current = (CatalogEntity) entity; + CatalogPO currentPO = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, + mapper -> mapper.selectCatalogMetaById(current.id())); + SessionUtils.doWithCommitAndFetchResult( + CatalogMetaMapper.class, + mapper -> + mapper.softDeleteCatalogMetasByCatalogId( + current.id(), currentPO.getCurrentVersion())); + return copyCatalogWithComment(current, "requested update"); + })); + } + + @TestTemplate + public void testNonCascadeDeleteRollsBackCatalogFence() throws IOException { + CatalogEntity catalog = + createCatalog( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofCatalog(metalakeName), + "catalog_non_empty", + auditInfo); + backend.insert(catalog, false); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalog.name()), + "schema", + auditInfo); + backend.insert(schema, false); + CatalogPO beforeDelete = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + + assertThrows( + NonEmptyEntityException.class, + () -> CatalogMetaService.getInstance().deleteCatalog(catalog.nameIdentifier(), false)); + + CatalogPO afterDelete = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { CatalogEntity catalog = @@ -303,6 +541,59 @@ public void testDeleteCatalogCascadeRemovesTagRelations() throws IOException { assertEquals(0, countActiveTagRelForMetadataObject(function.id(), "FUNCTION")); } + private List insertCatalogsConcurrently(CatalogEntity first, CatalogEntity second) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + try { + Future firstResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + CatalogMetaService.getInstance().insertCatalog(first, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + Future secondResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + CatalogMetaService.getInstance().insertCatalog(second, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(ready.await(30, TimeUnit.SECONDS)); + start.countDown(); + return Arrays.asList( + firstResult.get(30, TimeUnit.SECONDS), secondResult.get(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + executor.shutdownNow(); + } + } + + private CatalogEntity copyCatalogWithComment(CatalogEntity catalog, String comment) { + return CatalogEntity.builder() + .withId(catalog.id()) + .withName(catalog.name()) + .withNamespace(catalog.namespace()) + .withType(catalog.getType()) + .withProvider(catalog.getProvider()) + .withComment(comment) + .withProperties(catalog.getProperties()) + .withAuditInfo(auditInfo) + .build(); + } + private void associateTag(TagEntity tag, NameIdentifier ident, Entity.EntityType type) throws IOException { TagMetaService.getInstance() diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java index b1e6389a208..952416c1cf5 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestMetalakeMetaService.java @@ -25,14 +25,33 @@ import java.io.IOException; import java.time.Instant; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; +import org.apache.gravitino.exceptions.NoSuchEntityException; +import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.meta.CatalogEntity; +import org.apache.gravitino.meta.SchemaEntity; import org.apache.gravitino.meta.SchemaVersion; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; +import org.apache.gravitino.storage.relational.mapper.MetalakeMetaMapper; +import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper; +import org.apache.gravitino.storage.relational.po.MetalakePO; +import org.apache.gravitino.storage.relational.po.SchemaPO; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; +import org.apache.gravitino.utils.NamespaceUtil; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.TestTemplate; +import org.mockito.Mockito; public class TestMetalakeMetaService extends TestJDBCBackend { @@ -92,6 +111,310 @@ void testUpdateMetalakeWithNullableComment() throws IOException { backend.delete(metalake.nameIdentifier(), Entity.EntityType.METALAKE, false); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + MetalakePO oldPO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + BaseMetalake updatedMetalake = + BaseMetalake.builder() + .withId(metalake.id()) + .withName(metalake.name()) + .withAuditInfo(metalake.auditInfo()) + .withComment("updated") + .withProperties(metalake.properties()) + .withVersion(metalake.getVersion()) + .build(); + MetalakePO newPO = POConverters.updateMetalakePOWithVersion(oldPO, updatedMetalake); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> + mapper.softDeleteMetalakeMetaByMetalakeId( + metalake.id(), oldPO.getCurrentVersion())); + Assertions.assertEquals(1, updated); + Assertions.assertEquals(0, staleUpdate); + Assertions.assertEquals(0, staleDelete); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> + mapper.softDeleteMetalakeMetaByMetalakeId( + metalake.id(), newPO.getCurrentVersion())); + Assertions.assertEquals(1, deleted); + } + + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + + assertThrows( + OptimisticLockException.class, + () -> + MetalakeMetaService.getInstance() + .updateMetalake( + metalake.nameIdentifier(), + entity -> { + BaseMetalake current = (BaseMetalake) entity; + MetalakePO currentPO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.selectMetalakeMetaByName(current.name())); + BaseMetalake competingUpdate = + BaseMetalake.builder() + .withId(current.id()) + .withName(current.name()) + .withAuditInfo(current.auditInfo()) + .withComment("competing update") + .withProperties(current.properties()) + .withVersion(current.getVersion()) + .build(); + MetalakePO competingPO = + POConverters.updateMetalakePOWithVersion(currentPO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> mapper.updateMetalakeMeta(competingPO, currentPO)); + return BaseMetalake.builder() + .withId(current.id()) + .withName(current.name()) + .withAuditInfo(current.auditInfo()) + .withComment("requested update") + .withProperties(current.properties()) + .withVersion(current.getVersion()) + .build(); + })); + } + + @TestTemplate + public void testAlterReportsNoSuchWhenMetalakeIsDeletedConcurrently() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + + assertThrows( + NoSuchEntityException.class, + () -> + MetalakeMetaService.getInstance() + .updateMetalake( + metalake.nameIdentifier(), + entity -> { + BaseMetalake current = (BaseMetalake) entity; + MetalakePO currentPO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, + mapper -> mapper.selectMetalakeMetaById(current.id())); + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, + mapper -> + mapper.softDeleteMetalakeMetaByMetalakeId( + current.id(), currentPO.getCurrentVersion())); + return BaseMetalake.builder() + .withId(current.id()) + .withName(current.name()) + .withAuditInfo(current.auditInfo()) + .withComment("requested update") + .withProperties(current.properties()) + .withVersion(current.getVersion()) + .build(); + })); + } + + @TestTemplate + public void testDeleteReportsOptimisticLockConflict() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + MetalakePO stalePO = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + BaseMetalake competingUpdate = + BaseMetalake.builder() + .withId(metalake.id()) + .withName(metalake.name()) + .withAuditInfo(metalake.auditInfo()) + .withComment("competing update") + .withProperties(metalake.properties()) + .withVersion(metalake.getVersion()) + .build(); + MetalakePO competingPO = POConverters.updateMetalakePOWithVersion(stalePO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + MetalakeMetaMapper.class, mapper -> mapper.updateMetalakeMeta(competingPO, stalePO)); + + assertThrows( + OptimisticLockException.class, + () -> + SessionUtils.doMultipleWithCommit( + () -> + MetalakeMetaService.getInstance() + .deleteMetalakeWithVersion( + metalake.nameIdentifier(), + metalake.id(), + stalePO.getCurrentVersion()))); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + } + + @TestTemplate + public void testCascadeDeleteReportsConcurrentSchemaAlter() throws Exception { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + CatalogEntity catalog = createAndInsertCatalog(METALAKE_NAME, "catalog"); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(METALAKE_NAME, catalog.name()), + "schema", + AUDIT_INFO); + backend.insert(schema, false); + SchemaPO schemaBeforeDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + + ExecutorService executor = Executors.newSingleThreadExecutor(); + MetalakeMetaService service = Mockito.spy(MetalakeMetaService.getInstance()); + try { + Mockito.doAnswer( + invocation -> { + Assertions.assertEquals(metalake.id(), invocation.getArgument(0)); + List schemaPOs = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.listSchemaPOsByMetalakeId(metalake.id())); + SchemaPO observedSchemaPO = + schemaPOs.stream() + .filter(schemaPO -> schemaPO.getSchemaId().equals(schema.id())) + .findFirst() + .orElseThrow(); + SchemaEntity competingSchema = + SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withComment("competing update") + .withProperties(schema.properties()) + .withAuditInfo(schema.auditInfo()) + .build(); + SchemaPO competingSchemaPO = + POConverters.updateSchemaPOWithVersion(observedSchemaPO, competingSchema); + Future competingUpdate = + executor.submit( + () -> + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.updateSchemaMeta(competingSchemaPO, observedSchemaPO))); + Assertions.assertEquals(1, competingUpdate.get(30, TimeUnit.SECONDS)); + return schemaPOs; + }) + .when(service) + .listSchemaPOsForCascade(metalake.id()); + + assertThrows( + OptimisticLockException.class, + () -> service.deleteMetalake(metalake.nameIdentifier(), true)); + } finally { + executor.shutdownNow(); + } + + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + assertTrue(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + SchemaPO schemaAfterDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + Assertions.assertEquals( + schemaBeforeDelete.getCurrentVersion() + 1, schemaAfterDelete.getCurrentVersion()); + Assertions.assertEquals("competing update", schemaAfterDelete.getSchemaComment()); + } + + @TestTemplate + public void testConcurrentMetalakeCascadeAndSchemaCreateLeavesNoOrphan() throws Exception { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + CatalogEntity catalog = createAndInsertCatalog(METALAKE_NAME, "catalog"); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(METALAKE_NAME, catalog.name()), + "concurrent_schema", + AUDIT_INFO); + CountDownLatch catalogsLocked = new CountDownLatch(1); + CountDownLatch allowSchemaSnapshot = new CountDownLatch(1); + CountDownLatch createStarted = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(2); + MetalakeMetaService service = Mockito.spy(MetalakeMetaService.getInstance()); + try { + Mockito.doAnswer( + invocation -> { + catalogsLocked.countDown(); + assertTrue(allowSchemaSnapshot.await(30, TimeUnit.SECONDS)); + return invocation.callRealMethod(); + }) + .when(service) + .listSchemaPOsForCascade(metalake.id()); + + Future deleteResult = + executor.submit( + () -> { + try { + service.deleteMetalake(metalake.nameIdentifier(), true); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(catalogsLocked.await(30, TimeUnit.SECONDS)); + + Future createResult = + executor.submit( + () -> { + createStarted.countDown(); + try { + SchemaMetaService.getInstance().insertSchema(schema, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(createStarted.await(30, TimeUnit.SECONDS)); + assertThrows(TimeoutException.class, () -> createResult.get(500, TimeUnit.MILLISECONDS)); + + allowSchemaSnapshot.countDown(); + Throwable createFailure = createResult.get(30, TimeUnit.SECONDS); + Throwable deleteFailure = deleteResult.get(30, TimeUnit.SECONDS); + Assertions.assertInstanceOf(NoSuchEntityException.class, createFailure); + Assertions.assertNull(deleteFailure, () -> "Metalake cascade failed: " + deleteFailure); + } finally { + allowSchemaSnapshot.countDown(); + executor.shutdownNow(); + } + + assertFalse(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + assertFalse(backend.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG)); + assertFalse(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + } + + @TestTemplate + public void testNonCascadeDeleteRollsBackMetalakeFence() throws IOException { + BaseMetalake metalake = createAndInsertMakeLake(METALAKE_NAME); + createAndInsertCatalog(METALAKE_NAME, "catalog"); + MetalakePO beforeDelete = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + + assertThrows( + NonEmptyEntityException.class, + () -> MetalakeMetaService.getInstance().deleteMetalake(metalake.nameIdentifier(), false)); + + MetalakePO afterDelete = + SessionUtils.getWithoutCommit( + MetalakeMetaMapper.class, mapper -> mapper.selectMetalakeMetaByName(metalake.name())); + Assertions.assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(metalake.nameIdentifier(), Entity.EntityType.METALAKE)); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { // meta data creation diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java index a1e43144ec8..5310da2cf3f 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/service/TestSchemaMetaService.java @@ -28,15 +28,26 @@ import java.sql.SQLException; import java.sql.Statement; import java.time.Instant; +import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.stream.Collectors; import org.apache.gravitino.Entity; import org.apache.gravitino.EntityAlreadyExistsException; import org.apache.gravitino.NameIdentifier; import org.apache.gravitino.Namespace; +import org.apache.gravitino.exceptions.NoSuchEntityException; import org.apache.gravitino.exceptions.NonEmptyEntityException; +import org.apache.gravitino.exceptions.OptimisticLockException; +import org.apache.gravitino.meta.CatalogEntity; import org.apache.gravitino.meta.ColumnEntity; import org.apache.gravitino.meta.FilesetEntity; import org.apache.gravitino.meta.FunctionEntity; @@ -49,7 +60,13 @@ import org.apache.gravitino.rel.types.Types; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.storage.relational.TestJDBCBackend; +import org.apache.gravitino.storage.relational.mapper.CatalogMetaMapper; +import org.apache.gravitino.storage.relational.mapper.SchemaMetaMapper; +import org.apache.gravitino.storage.relational.po.CatalogPO; +import org.apache.gravitino.storage.relational.po.SchemaPO; import org.apache.gravitino.storage.relational.session.SqlSessionFactoryHelper; +import org.apache.gravitino.storage.relational.utils.POConverters; +import org.apache.gravitino.storage.relational.utils.SessionUtils; import org.apache.gravitino.utils.NameIdentifierUtil; import org.apache.gravitino.utils.NamespaceUtil; import org.apache.ibatis.session.SqlSession; @@ -81,6 +98,192 @@ public void testInsertAlreadyExistsException() throws IOException { assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(schemaCopy, false)); } + @TestTemplate + public void testInsertSchemaLocksCatalogWithoutChangingVersion() throws IOException { + createAndInsertMakeLake(metalakeName); + CatalogEntity catalog = createAndInsertCatalog(metalakeName, catalogName); + CatalogPO beforeInsert = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_fence", + AUDIT_INFO); + backend.insert(schema, false); + + CatalogPO afterInsert = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + Assertions.assertEquals(beforeInsert.getCurrentVersion(), afterInsert.getCurrentVersion()); + Assertions.assertEquals(beforeInsert.getLastVersion(), afterInsert.getLastVersion()); + + SchemaEntity duplicate = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + schema.name(), + AUDIT_INFO); + assertThrows(EntityAlreadyExistsException.class, () -> backend.insert(duplicate, false)); + + CatalogPO afterFailure = + SessionUtils.getWithoutCommit( + CatalogMetaMapper.class, mapper -> mapper.selectCatalogMetaById(catalog.id())); + Assertions.assertEquals(afterInsert.getCurrentVersion(), afterFailure.getCurrentVersion()); + Assertions.assertEquals(afterInsert.getLastVersion(), afterFailure.getLastVersion()); + } + + @TestTemplate + public void testEntityCreateWaitsForConcurrentSchemaDelete() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_for_entity_lock", + AUDIT_INFO); + backend.insert(schema, false); + SchemaPO observedSchemaPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + + CountDownLatch schemaDeleteLocked = new CountDownLatch(1); + CountDownLatch allowDeleteCommit = new CountDownLatch(1); + CountDownLatch entityCreateStarted = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(2); + Future deleteResult = + executor.submit( + () -> { + try { + SessionUtils.doMultipleWithCommit( + () -> { + int deleted = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + observedSchemaPO.getSchemaId(), + observedSchemaPO.getCurrentVersion())); + Assertions.assertEquals(1, deleted); + schemaDeleteLocked.countDown(); + try { + assertTrue(allowDeleteCommit.await(30, TimeUnit.SECONDS)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + }); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + try { + assertTrue(schemaDeleteLocked.await(30, TimeUnit.SECONDS)); + NameIdentifier tableIdentifier = + NameIdentifier.of(metalakeName, catalogName, schema.name(), "new_table"); + Future createResult = + executor.submit( + () -> { + entityCreateStarted.countDown(); + try { + SessionUtils.doMultipleWithCommit( + () -> + SchemaMetaService.getInstance() + .lockSchemaForEntityWrite( + tableIdentifier, + observedSchemaPO.getSchemaId(), + observedSchemaPO.getCatalogId(), + observedSchemaPO.getMetalakeId())); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(entityCreateStarted.await(30, TimeUnit.SECONDS)); + assertThrows(TimeoutException.class, () -> createResult.get(500, TimeUnit.MILLISECONDS)); + + allowDeleteCommit.countDown(); + Assertions.assertNull(deleteResult.get(30, TimeUnit.SECONDS)); + Assertions.assertInstanceOf( + NoSuchEntityException.class, createResult.get(30, TimeUnit.SECONDS)); + } finally { + allowDeleteCommit.countDown(); + executor.shutdownNow(); + } + } + + @TestTemplate + public void testConcurrentSameNameSchemaCreateReportsAlreadyExists() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity first = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema", + AUDIT_INFO); + SchemaEntity second = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + first.name(), + AUDIT_INFO); + + List results = insertSchemasConcurrently(first, second); + Assertions.assertEquals(1, results.stream().filter(Objects::isNull).count()); + Throwable failure = results.stream().filter(Objects::nonNull).findFirst().orElseThrow(); + Assertions.assertTrue( + failure instanceof EntityAlreadyExistsException, + () -> "Expected EntityAlreadyExistsException, but got " + failure); + } + + @TestTemplate + public void testConcurrentDifferentSchemaCreatesBothSucceed() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity first = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema_1", + AUDIT_INFO); + SchemaEntity second = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_schema_2", + AUDIT_INFO); + + List results = insertSchemasConcurrently(first, second); + Assertions.assertTrue( + results.stream().allMatch(Objects::isNull), + () -> "Concurrent schema creates failed: " + results); + } + + @TestTemplate + public void testConcurrentSameSchemaDeletesAreIdempotent() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "concurrent_delete_schema", + AUDIT_INFO); + backend.insert(schema, false); + + List results = + deleteSchemasConcurrently(schema.nameIdentifier(), schema.nameIdentifier()); + Assertions.assertEquals(1, results.stream().filter(Objects::isNull).count()); + Throwable loser = results.stream().filter(Objects::nonNull).findFirst().orElseThrow(); + Assertions.assertTrue( + loser instanceof NoSuchEntityException, + () -> "Expected an idempotent missing result, but got " + loser); + } + @TestTemplate public void testUpdateAlreadyExistsException() throws IOException { createAndInsertMakeLake(metalakeName); @@ -145,6 +348,124 @@ public void testUpdateSchemaCommentFromNull() throws IOException { Assertions.assertEquals("schema comment updated", updatedSchema.comment()); } + @TestTemplate + public void testAlterAndDeleteUseCurrentVersion() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_occ", + AUDIT_INFO); + backend.insert(schema, false); + SchemaPO oldPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + SchemaEntity updatedSchema = + SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withAuditInfo(schema.auditInfo()) + .withComment("updated") + .withProperties(schema.properties()) + .build(); + SchemaPO newPO = POConverters.updateSchemaPOWithVersion(oldPO, updatedSchema); + + int updated = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, mapper -> mapper.updateSchemaMeta(newPO, oldPO)); + int staleUpdate = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, mapper -> mapper.updateSchemaMeta(newPO, oldPO)); + int staleDelete = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + schema.id(), oldPO.getCurrentVersion())); + Assertions.assertEquals(1, updated); + Assertions.assertEquals(0, staleUpdate); + Assertions.assertEquals(0, staleDelete); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + int deleted = + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + schema.id(), newPO.getCurrentVersion())); + Assertions.assertEquals(1, deleted); + } + + @TestTemplate + public void testAlterReportsOptimisticLockConflict() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_alter_conflict", + AUDIT_INFO); + backend.insert(schema, false); + + assertThrows( + OptimisticLockException.class, + () -> + SchemaMetaService.getInstance() + .updateSchema( + schema.nameIdentifier(), + entity -> { + SchemaEntity current = (SchemaEntity) entity; + SchemaPO currentPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.selectSchemaMetaById(current.id())); + SchemaEntity competingUpdate = + copySchemaWithComment(current, "competing update"); + SchemaPO competingPO = + POConverters.updateSchemaPOWithVersion(currentPO, competingUpdate); + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> mapper.updateSchemaMeta(competingPO, currentPO)); + return copySchemaWithComment(current, "requested update"); + })); + } + + @TestTemplate + public void testAlterReportsNoSuchWhenSchemaIsDeletedConcurrently() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_alter_deleted", + AUDIT_INFO); + backend.insert(schema, false); + + assertThrows( + NoSuchEntityException.class, + () -> + SchemaMetaService.getInstance() + .updateSchema( + schema.nameIdentifier(), + entity -> { + SchemaEntity current = (SchemaEntity) entity; + SchemaPO currentPO = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> mapper.selectSchemaMetaById(current.id())); + SessionUtils.doWithCommitAndFetchResult( + SchemaMetaMapper.class, + mapper -> + mapper.softDeleteSchemaMetaBySchemaIdAndVersion( + current.id(), currentPO.getCurrentVersion())); + return copySchemaWithComment(current, "requested update"); + })); + } + @TestTemplate public void testMetaLifeCycleFromCreationToDeletion() throws IOException { createAndInsertMakeLake(metalakeName); @@ -215,16 +536,77 @@ public void testDeleteSchemaNonCascadingFailsWhenTopicExists() throws IOExceptio topicName, AUDIT_INFO); topicMetaService.insertTopic(topic, false); + SchemaPO beforeDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); Assertions.assertThrows( NonEmptyEntityException.class, () -> schemaMetaService.deleteSchema(schema.nameIdentifier(), false), "Non-cascading delete must fail when dependent topics exist."); + SchemaPO afterDelete = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, mapper -> mapper.selectSchemaMetaById(schema.id())); + Assertions.assertEquals(beforeDelete.getCurrentVersion(), afterDelete.getCurrentVersion()); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + assertTrue(backend.exists(topic.nameIdentifier(), Entity.EntityType.TOPIC)); + topicMetaService.deleteTopic(topic.nameIdentifier()); schemaMetaService.deleteSchema(schema.nameIdentifier(), false); } + @TestTemplate + public void testDeleteSchemaNonCascadingFailsWhenViewExists() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_with_view", + AUDIT_INFO); + SchemaMetaService.getInstance().insertSchema(schema, false); + ViewEntity view = + createViewEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofView(metalakeName, catalogName, schema.name()), + "dependent_view"); + ViewMetaService.getInstance().insertView(view, false); + + assertThrows( + NonEmptyEntityException.class, + () -> SchemaMetaService.getInstance().deleteSchema(schema.nameIdentifier(), false)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + assertTrue(backend.exists(view.nameIdentifier(), Entity.EntityType.VIEW)); + } + + @TestTemplate + public void testDeleteSchemaNonCascadingFailsWhenFunctionExists() throws IOException { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaEntity schema = + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + "schema_with_function", + AUDIT_INFO); + SchemaMetaService.getInstance().insertSchema(schema, false); + FunctionEntity function = + createFunctionEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofFunction(metalakeName, catalogName, schema.name()), + "dependent_function", + AUDIT_INFO); + FunctionMetaService.getInstance().insertFunction(function, false); + + assertThrows( + NonEmptyEntityException.class, + () -> SchemaMetaService.getInstance().deleteSchema(schema.nameIdentifier(), false)); + assertTrue(backend.exists(schema.nameIdentifier(), Entity.EntityType.SCHEMA)); + assertTrue(backend.exists(function.nameIdentifier(), Entity.EntityType.FUNCTION)); + } + @TestTemplate public void testInsertHierarchicalSchemaCreatesAncestorsAndLeaf() throws IOException { createAndInsertMakeLake(metalakeName); @@ -338,6 +720,42 @@ public void testDeleteHierarchicalSchemaCascadeRemovesDescendantsAndChildren() NameIdentifier.of(metalakeName, catalogName, "anc_a"), Entity.EntityType.SCHEMA)); } + @TestTemplate + public void testOverlappingHierarchicalSchemaDeletesDoNotDeadlock() throws Exception { + createAndInsertMakeLake(metalakeName); + createAndInsertCatalog(metalakeName, catalogName); + SchemaMetaService schemaMetaService = SchemaMetaService.getInstance(); + String firstLeaf = "overlap_a:overlap_b:leaf_c"; + String secondLeaf = "overlap_a:overlap_b:leaf_d"; + schemaMetaService.insertSchema( + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + firstLeaf, + AUDIT_INFO), + false); + schemaMetaService.insertSchema( + createSchemaEntity( + RandomIdGenerator.INSTANCE.nextId(), + NamespaceUtil.ofSchema(metalakeName, catalogName), + secondLeaf, + AUDIT_INFO), + false); + + NameIdentifier ancestor = NameIdentifier.of(metalakeName, catalogName, "overlap_a:overlap_b"); + NameIdentifier descendant = NameIdentifier.of(metalakeName, catalogName, firstLeaf); + List results = deleteSchemasConcurrently(ancestor, descendant); + Assertions.assertTrue( + results.stream() + .allMatch(result -> result == null || result instanceof NoSuchEntityException), + () -> "Overlapping cascade deletes produced an unexpected failure: " + results); + Assertions.assertFalse(backend.exists(ancestor, Entity.EntityType.SCHEMA)); + Assertions.assertFalse(backend.exists(descendant, Entity.EntityType.SCHEMA)); + Assertions.assertFalse( + backend.exists( + NameIdentifier.of(metalakeName, catalogName, secondLeaf), Entity.EntityType.SCHEMA)); + } + @TestTemplate public void testDeleteSchemaCascadeRemovesTagRelations() throws IOException { createAndInsertMakeLake(metalakeName); @@ -507,14 +925,24 @@ public void testInsertHierarchicalSecondLeafReusesAncestorsWithoutUpsert() throw .build(); schemaMetaService.insertSchema(first, false); - long idA = - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorA)) - .id(); - long idAB = - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorAB)) - .id(); + SchemaPO ancestorAPOBefore = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorA)) + .id())); + SchemaPO ancestorABPOBefore = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorAB)) + .id())); SchemaEntity second = SchemaEntity.builder() @@ -527,16 +955,113 @@ public void testInsertHierarchicalSecondLeafReusesAncestorsWithoutUpsert() throw .build(); schemaMetaService.insertSchema(second, false); + SchemaPO ancestorAPOAfter = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorA)) + .id())); + SchemaPO ancestorABPOAfter = + SessionUtils.getWithoutCommit( + SchemaMetaMapper.class, + mapper -> + mapper.selectSchemaMetaById( + schemaMetaService + .getSchemaByIdentifier( + NameIdentifier.of(metalakeName, catalogName, ancestorAB)) + .id())); + Assertions.assertEquals(ancestorAPOBefore.getSchemaId(), ancestorAPOAfter.getSchemaId()); + Assertions.assertEquals(ancestorABPOBefore.getSchemaId(), ancestorABPOAfter.getSchemaId()); Assertions.assertEquals( - idA, - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorA)) - .id()); + ancestorAPOBefore.getCurrentVersion(), ancestorAPOAfter.getCurrentVersion()); Assertions.assertEquals( - idAB, - schemaMetaService - .getSchemaByIdentifier(NameIdentifier.of(metalakeName, catalogName, ancestorAB)) - .id()); + ancestorABPOBefore.getCurrentVersion(), ancestorABPOAfter.getCurrentVersion()); + } + + private List insertSchemasConcurrently(SchemaEntity first, SchemaEntity second) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + try { + Future firstResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + SchemaMetaService.getInstance().insertSchema(first, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + Future secondResult = + executor.submit( + () -> { + ready.countDown(); + start.await(); + try { + SchemaMetaService.getInstance().insertSchema(second, false); + return null; + } catch (Throwable throwable) { + return throwable; + } + }); + assertTrue(ready.await(30, TimeUnit.SECONDS)); + start.countDown(); + return Arrays.asList( + firstResult.get(30, TimeUnit.SECONDS), secondResult.get(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + executor.shutdownNow(); + } + } + + private List deleteSchemasConcurrently(NameIdentifier first, NameIdentifier second) + throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(2); + CountDownLatch ready = new CountDownLatch(2); + CountDownLatch start = new CountDownLatch(1); + try { + Future firstResult = + executor.submit(() -> deleteSchemaAfterStart(first, ready, start)); + Future secondResult = + executor.submit(() -> deleteSchemaAfterStart(second, ready, start)); + assertTrue(ready.await(30, TimeUnit.SECONDS)); + start.countDown(); + return Arrays.asList( + firstResult.get(30, TimeUnit.SECONDS), secondResult.get(30, TimeUnit.SECONDS)); + } finally { + start.countDown(); + executor.shutdownNow(); + } + } + + private Throwable deleteSchemaAfterStart( + NameIdentifier identifier, CountDownLatch ready, CountDownLatch start) { + ready.countDown(); + try { + start.await(); + SchemaMetaService.getInstance().deleteSchema(identifier, true); + return null; + } catch (Throwable throwable) { + return throwable; + } + } + + private SchemaEntity copySchemaWithComment(SchemaEntity schema, String comment) { + return SchemaEntity.builder() + .withId(schema.id()) + .withName(schema.name()) + .withNamespace(schema.namespace()) + .withComment(comment) + .withProperties(schema.properties()) + .withAuditInfo(schema.auditInfo()) + .build(); } private void associateTag(TagEntity tag, NameIdentifier ident, Entity.EntityType type) diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestExceptionUtils.java b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestExceptionUtils.java new file mode 100644 index 00000000000..0d489be6882 --- /dev/null +++ b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestExceptionUtils.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.storage.relational.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.gravitino.Entity; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.exceptions.OptimisticLockException; +import org.junit.jupiter.api.Test; + +public class TestExceptionUtils { + + @Test + public void testConcurrentModificationMessage() { + OptimisticLockException e = + ExceptionUtils.concurrentModification( + Entity.EntityType.CATALOG, NameIdentifier.of("m1", "c1")); + + assertEquals( + "The catalog m1.c1 was modified concurrently; retry the operation", e.getMessage()); + } + + @Test + public void testConcurrentChildModificationMessage() { + OptimisticLockException e = + ExceptionUtils.concurrentChildModification( + Entity.EntityType.SCHEMA, Entity.EntityType.CATALOG, NameIdentifier.of("m1", "c1")); + + assertEquals( + "A schema under catalog m1.c1 was modified concurrently; retry the operation", + e.getMessage()); + } +} diff --git a/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java index 9ce171d8782..994432dcaa5 100644 --- a/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java +++ b/core/src/test/java/org/apache/gravitino/storage/relational/utils/TestPOConverters.java @@ -665,6 +665,8 @@ public void testUpdateMetalakePOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getMetalakeComment()); } @@ -679,6 +681,8 @@ public void testUpdateCatalogPOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getCatalogComment()); } @@ -696,6 +700,8 @@ public void testUpdateSchemaPOVersion() { assertEquals(1, initPO.getCurrentVersion()); assertEquals(1, initPO.getLastVersion()); assertEquals(0, initPO.getDeletedAt()); + assertEquals(2, updatePO.getCurrentVersion()); + assertEquals(2, updatePO.getLastVersion()); assertEquals("this is test2", updatePO.getSchemaComment()); }