From b538e55409e43070880be96ac055d293e877b3d7 Mon Sep 17 00:00:00 2001 From: Yingjian Wu Date: Tue, 9 Jul 2024 13:35:47 -0400 Subject: [PATCH] rename with 2 phase commit idea --- .../ParentChildRelMetadataService.java | 16 +- .../ParentChildRelMetadataServiceSpec.groovy | 209 ++++++++++++++++-- .../main/services/impl/TableServiceImpl.java | 39 +++- .../services/impl/TableServiceImplSpec.groovy | 66 ++++-- .../MySqlParentChildRelMetaDataService.java | 116 ++++++---- 5 files changed, 364 insertions(+), 82 deletions(-) diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java index 0ddfcbad..6f5af8d0 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java @@ -5,6 +5,8 @@ import com.netflix.metacat.common.server.model.ChildInfo; import com.netflix.metacat.common.server.model.ParentInfo; import com.netflix.metacat.common.server.properties.ParentChildRelationshipProperties; +import org.apache.commons.lang3.tuple.Pair; +import java.util.Optional; import java.util.Set; @@ -59,13 +61,16 @@ void deleteParentChildRelation( /** * Renames `oldName` to `newName` in the parentChildRelationship store. * This involves two steps: - * 1. Rename all records where the child is `oldName` to `newName` - * 2. Rename all records where the parent is `oldName` to `newName` + * 1. duplicate all records where the child is `oldName` with childName = `newName` + * 2. duplicate all records where the parent is `oldName` with parentName `newName` * * @param oldName the current name to be renamed * @param newName the new name to rename to + * @return return a pair of set, + * where the first set represents the affected parent_uuid with parent = oldName + * and the second set represents the affected child_uuid with child = oldName */ - void rename( + Pair, Set> renameSoftInsert( QualifiedName oldName, QualifiedName newName ); @@ -75,10 +80,13 @@ void rename( * This involves two steps: * 1. drop all records where the child column = `name` * 2. drop all records where the parent column = `name` + * * Note if uuids are specified, it is going to drop the table with that name with the corresponding uuids * @param name the name of the entity to drop + * @param uuids the uuids to drop, where the first pair is the parent uuid and second pair is the child uuid */ void drop( - QualifiedName name + QualifiedName name, + Optional, Set>> uuids ); /** diff --git a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy index c199b12f..08f47b7c 100644 --- a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy +++ b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy @@ -229,9 +229,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) when: - service.rename(parent, newParent) + def pair = service.renameSoftInsert(parent, newParent) + service.drop(parent, Optional.of(pair)) then: + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() + // Test Old Parent Name assert service.getParents(parent).isEmpty() assert service.getChildren(parent).isEmpty() @@ -254,10 +258,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // rename back when: - service.rename(newParent, parent) + pair = service.renameSoftInsert(newParent, parent) + service.drop(newParent, Optional.of(pair)) child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set then: + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() // Test new Parent Name assert service.getParents(newParent).isEmpty() assert service.getChildren(newParent).isEmpty() @@ -290,12 +297,19 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child2, child2UUID, type, props) def newParent = QualifiedName.ofTable(catalog, database, "np") def child_parent_expected = [new ParentInfo(newParent.toString(), type, parentUUID)] as Set + def newParent_children_expected = [ + new ChildInfo(child1.toString(), type, child1UUID), + new ChildInfo(child2.toString(), type, child2UUID) + ] as Set when: - service.rename(parent, newParent) + def pair = service.renameSoftInsert(parent, newParent) + service.drop(parent, Optional.of(pair)) then: // Test Old Parent Name + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() assert service.getParents(parent).isEmpty() assert service.getChildren(parent).isEmpty() assert !service.isChildTable(parent) @@ -303,15 +317,9 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // Test New Parent Name assert service.getParents(newParent).isEmpty() - def newParent_children_expected = [ - new ChildInfo(child1.toString(), type, child1UUID), - new ChildInfo(child2.toString(), type, child2UUID), - ] as Set assert service.getChildren(newParent) == newParent_children_expected assert !service.isChildTable(newParent) assert service.isParentTable(newParent) - - then: // Test Child1 assert service.getParents(child1) == child_parent_expected assert service.isChildTable(child1) @@ -333,8 +341,12 @@ class ParentChildRelMetadataServiceSpec extends Specification{ def newChild = QualifiedName.ofTable(catalog, database, "nc") when: - service.rename(child, newChild) + def pair = service.renameSoftInsert(child, newChild) + service.drop(child, Optional.of(pair)) + then: + assert pair.getLeft().isEmpty() + assert pair.getRight().size() == 1 // Test Parent assert service.getParents(parent).isEmpty() def parent_children_expected = [new ChildInfo(newChild.toString(), type, childUUID)] as Set @@ -357,10 +369,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // rename back when: - service.rename(newChild, child) + pair = service.renameSoftInsert(newChild, child) + service.drop(newChild, Optional.of(pair)) parent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set then: + assert pair.getLeft().isEmpty() + assert pair.getRight().size() == 1 // Test Parent assert service.getParents(parent).isEmpty() assert parent_children_expected == service.getChildren(parent) @@ -390,7 +405,7 @@ class ParentChildRelMetadataServiceSpec extends Specification{ def type = "clone"; service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) when: - service.drop(child) + service.drop(child, Optional.empty()) then: // Test Parent @@ -417,8 +432,9 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) when: - service.rename(child, newChild) - service.drop(newChild) + def pair = service.renameSoftInsert(child, newChild) + service.drop(child, Optional.of(pair)) + service.drop(newChild, Optional.empty()) then: // Test Parent @@ -460,14 +476,14 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // rename to an existing parent when: - service.rename(parent1, parent2) + service.renameSoftInsert(parent1, parent2) then: def e = thrown(Exception) assert e.message.contains("is already a parent table") // rename to an existing child when: - service.rename(child2, child1) + service.renameSoftInsert(child2, child1) then: e = thrown(Exception) assert e.message.contains("is already a child table") @@ -618,4 +634,165 @@ class ParentChildRelMetadataServiceSpec extends Specification{ 1 | "CLONE,5" | "CLONE,test,3;OTHER,other,2"| "CLONE,testhive/test/parent,2" | 2 1 | "CLONE,5;Other,3" | "CLONE,test,3;CLONE,other,2"| "CLONE,testhive/test/parent,2;CLONE,testhive/test/other,2" | 2 } + def "Test Parent Rename failed and remove newName"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone"; + def newParent = QualifiedName.ofTable(catalog, database, "np") + service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) + + when: + def pair = service.renameSoftInsert(parent, newParent) + service.drop(newParent, Optional.of(pair)) + + then: + // Test New Parent Name + assert service.getParents(newParent).isEmpty() + assert service.getChildren(newParent).isEmpty() + + // Test Old Parent Name + assert service.getParents(parent).isEmpty() + def newParent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set + assert service.getChildren(parent) == newParent_children_expected + + // Test Child + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + assert child_parent_expected == service.getParents(child) + assert service.getChildren(child).isEmpty() + } + + def "Test Child Rename failed and remove newName"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) + def newChild = QualifiedName.ofTable(catalog, database, "nc") + + when: + def pair = service.renameSoftInsert(child, newChild) + service.drop(newChild, Optional.of(pair)) + then: + // Test Parent + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test New Child + assert service.getParents(newChild).isEmpty() + assert service.getChildren(newChild).isEmpty() + + // Test Child + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + assert child_parent_expected == service.getParents(child) + assert service.getChildren(child).isEmpty() + } + + def "Test rename to an existing table in the parent child rel service"() { + given: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) + def newParentQualifiedName = QualifiedName.ofTable(catalog, database, "np_name") + def newChildQualifiedName = QualifiedName.ofTable(catalog, database, "nc_name") + insertNewParentChildRecord(newParentQualifiedName.toString(), "np_uuid", newChildQualifiedName.toString(), "nc_uuid", "random") + + when: + service.renameSoftInsert(parent, newParentQualifiedName) + then: + def e = thrown(RuntimeException) + assert e.message.contains("is already a parent table") + + when: + service.renameSoftInsert(child, newChildQualifiedName) + + then: + e = thrown(RuntimeException) + assert e.message.contains("is already a child table") + } + + // Add a test where between rename and drop old/new name with the same name but different uuid is added + // only those get from rename should be drop + // This case is not possible in reality but wanted to add a test to prove this + def "Simulate Rename child: drop only impacted uuids"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) + def newChild = QualifiedName.ofTable(catalog, database, "nc") + + when: + def pair = service.renameSoftInsert(child, newChild) + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + // Add a record with the same child name but different uuid and since this record is added after rename + // this record should not be dropped during the service.drop + insertNewParentChildRecord(parent.toString(), parentUUID, child.toString(), "c_uuid2", type) + service.drop(child, Optional.of(pair)) + then: + // Test Parent + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(newChild.toString(), type, childUUID), + new ChildInfo(child.toString(), type, "c_uuid2")] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test new child + assert service.getParents(newChild) == child_parent_expected + assert service.getChildren(newChild).isEmpty() + + // Test old child + assert service.getParents(child) == child_parent_expected + assert service.getChildren(child).isEmpty() + } + + // Add a test where between rename and drop old/new name same name but different uuid is added + // only those get from rename should be drop + // This case is not possible in reality but wanted to add a test to prove this + def "Simulate Rename Parent: drop only impacted uuids"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type, props) + def newParent = QualifiedName.ofTable(catalog, database, "np") + + when: + def pair = service.renameSoftInsert(parent, newParent) + // Add a record with the same parent name but different uuid and since this record is added after rename + // this record should not be dropped during the service.drop + insertNewParentChildRecord(parent.toString(), "p_uuid2", child.toString(), "c_uuid1", "clone") + service.drop(parent, Optional.of(pair)) + then: + + // Test p + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(child.toString(), "clone", "c_uuid1")] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test np + assert service.getParents(newParent).isEmpty() + def new_parent_children_expected = [new ChildInfo(child.toString(), "clone", childUUID)] as Set + assert new_parent_children_expected == service.getChildren(newParent) + + // Test child + assert service.getChildren((child)).isEmpty() + def child_parent_expected = [ + new ParentInfo(newParent.toString(), "clone", parentUUID), + new ParentInfo(parent.toString(), "clone", "p_uuid2"), + ] as Set + assert service.getParents(child) == child_parent_expected + } + } diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java index df784eab..a4124abc 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java @@ -69,6 +69,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.Pair; + import javax.annotation.Nullable; import java.util.List; import java.util.Map; @@ -418,7 +420,7 @@ public TableDto deleteAndReturn(final QualifiedName name, final boolean isMView) } try { - parentChildRelMetadataService.drop(name); + parentChildRelMetadataService.drop(name, Optional.empty()); } catch (Exception e) { log.error("parentChildRelMetadataService: Failed to drop relation for table={}", name, e); } @@ -599,23 +601,42 @@ public void rename( } } - // Before rename, first rename its parent child relation - parentChildRelMetadataService.rename(oldName, newName); - + // Before rename, first + // duplicate record with newName in parent child relation and get all uuids associated with this newName + // that are impacted so later during + // either dropping the oldName after successful connectorTableServiceProxy.rename + // or dropping the newName after failed connectorTableServiceProxy.rename + // we only touch records with those specific uuids + final Pair, Set> parentChildUuidsPair = + parentChildRelMetadataService.renameSoftInsert(oldName, newName); try { connectorTableServiceProxy.rename(oldName, newName, isMView); } catch (Exception e) { try { - // if rename operation fail, rename back the parent child relation - parentChildRelMetadataService.rename(newName, oldName); + // if rename operation fail, remove the soft inserted record + if (!parentChildUuidsPair.getLeft().isEmpty() || !parentChildUuidsPair.getRight().isEmpty()) { + parentChildRelMetadataService.drop(newName, Optional.of(parentChildUuidsPair)); + } } catch (Exception renameException) { - log.error("parentChildRelMetadataService: Failed to rename parent child relation " + log.error("parentChildRelMetadataService: Fail to rename parent child relation " + "after table fail to rename from {} to {} " - + "with the following parameters oldName={} to newName={}", - oldName, newName, oldName, newName, renameException); + + "with the following parameters oldName={} to newName={} and uuid pairs = {}", + oldName, newName, oldName, newName, parentChildUuidsPair, renameException); } throw e; } + // if rename everything succeeds, remove records with the oldName + try { + if (!parentChildUuidsPair.getLeft().isEmpty() || !parentChildUuidsPair.getRight().isEmpty()) { + parentChildRelMetadataService.drop(oldName, Optional.of(parentChildUuidsPair)); + } + } catch (Exception e) { + log.error("parentChildRelMetadataService: Fail to rename parent child relation " + + "after table successfully rename from {} to {} " + + "with the following parameters oldName={} to newName={} and uuid pairs = {}", + oldName, newName, oldName, newName, parentChildUuidsPair, e); + } + userMetadataService.renameDefinitionMetadataKey(oldName, newName); tagService.renameTableTags(oldName, newName.getTableName()); diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy index 5aedea82..d4c14288 100644 --- a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy @@ -53,7 +53,8 @@ import com.netflix.spectator.api.DefaultRegistry import com.netflix.spectator.api.NoopRegistry import spock.lang.Specification import spock.lang.Unroll -import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataService; +import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataService +import org.apache.commons.lang3.tuple.Pair /** * Tests for the TableServiceImpl. @@ -417,16 +418,22 @@ class TableServiceImplSpec extends Specification { given: def oldName = QualifiedName.ofTable("clone", "clone", "oldChild") def newName = QualifiedName.ofTable("clone", "clone", "newChild") + def set1 = new HashSet(['uuid']) + def set2 = new HashSet(['uuid']) + def pair = Pair.of(set1, set2) + def emptyPair = Pair.of(new HashSet(), new HashSet()) + def leftPair = Pair.of(set1, new HashSet()) + def rightPair = Pair.of(new HashSet(), set2) + // mock when rename fail and revert happen when: service.rename(oldName, newName, false) - then: 1 * config.isParentChildRenameEnabled() >> true 1 * config.getNoTableRenameOnTags() >> [] - 1 * parentChildRelSvc.rename(oldName, newName) + 1 * parentChildRelSvc.renameSoftInsert(oldName, newName) >> pair 1 * connectorTableServiceProxy.rename(oldName, newName, _) >> {throw new RuntimeException("Fail to rename")} - 1 * parentChildRelSvc.rename(newName, oldName) + 1 * parentChildRelSvc.drop(newName, Optional.of(pair)) thrown(RuntimeException) // mock when rename parent child relation is disabled and the table is a child table @@ -436,9 +443,11 @@ class TableServiceImplSpec extends Specification { 1 * config.getNoTableRenameOnTags() >> [] 1 * config.isParentChildRenameEnabled() >> false 1 * parentChildRelSvc.isChildTable(oldName) >> true + 0 * parentChildRelSvc.renameSoftInsert(oldName, newName) 0 * parentChildRelSvc.isParentTable(oldName) - 0 * parentChildRelSvc.rename(oldName, newName) + 0 * parentChildRelSvc.renameSoftInsert(oldName, newName) 0 * connectorTableServiceProxy.rename(oldName, newName, _) + 0 * parentChildRelSvc.drop(_, _) def e = thrown(RuntimeException) assert e.message.contains("is currently disabled") @@ -450,8 +459,9 @@ class TableServiceImplSpec extends Specification { 1 * config.isParentChildRenameEnabled() >> false 1 * parentChildRelSvc.isChildTable(oldName) >> false 1 * parentChildRelSvc.isParentTable(oldName) >> true - 0 * parentChildRelSvc.rename(oldName, newName) + 0 * parentChildRelSvc.renameSoftInsert(oldName, newName) 0 * connectorTableServiceProxy.rename(oldName, newName, _) + 0 * parentChildRelSvc.drop(_, _) e = thrown(RuntimeException) assert e.message.contains("is currently disabled") @@ -463,11 +473,38 @@ class TableServiceImplSpec extends Specification { 1 * config.isParentChildRenameEnabled() >> false 1 * parentChildRelSvc.isChildTable(oldName) >> false 1 * parentChildRelSvc.isParentTable(oldName) >> false - 1 * parentChildRelSvc.rename(oldName, newName) + 1 * parentChildRelSvc.renameSoftInsert(oldName, newName) >> emptyPair + 1 * connectorTableServiceProxy.rename(oldName, newName, _) + 0 * parentChildRelSvc.drop(oldName, _) + noExceptionThrown() + + // mock normal success case that is not a parent nor child table + when: + service.rename(oldName, newName, false) + then: + 1 * config.getNoTableRenameOnTags() >> [] + 1 * config.isParentChildRenameEnabled() >> true + 0 * parentChildRelSvc.isChildTable(oldName) + 0 * parentChildRelSvc.isParentTable(oldName) + 1 * parentChildRelSvc.renameSoftInsert(oldName, newName) >> emptyPair + 1 * connectorTableServiceProxy.rename(oldName, newName, _) + 0 * parentChildRelSvc.drop(oldName, Optional.of(emptyPair)) + noExceptionThrown() + + // mock normal success case that is a parent + when: + service.rename(oldName, newName, false) + then: + 1 * config.getNoTableRenameOnTags() >> [] + 1 * config.isParentChildRenameEnabled() >> true + 0 * parentChildRelSvc.isChildTable(oldName) + 0 * parentChildRelSvc.isParentTable(oldName) + 1 * parentChildRelSvc.renameSoftInsert(oldName, newName) >> leftPair 1 * connectorTableServiceProxy.rename(oldName, newName, _) + 1 * parentChildRelSvc.drop(oldName, Optional.of(leftPair)) noExceptionThrown() - // mock normal success case + // mock normal success case that is a child table when: service.rename(oldName, newName, false) then: @@ -475,8 +512,9 @@ class TableServiceImplSpec extends Specification { 1 * config.isParentChildRenameEnabled() >> true 0 * parentChildRelSvc.isChildTable(oldName) 0 * parentChildRelSvc.isParentTable(oldName) - 1 * parentChildRelSvc.rename(oldName, newName) + 1 * parentChildRelSvc.renameSoftInsert(oldName, newName) >> rightPair 1 * connectorTableServiceProxy.rename(oldName, newName, _) + 1 * parentChildRelSvc.drop(oldName, Optional.of(rightPair)) noExceptionThrown() } @@ -508,7 +546,7 @@ class TableServiceImplSpec extends Specification { 1 * parentChildRelSvc.getChildren(name) 1 * config.getNoTableDeleteOnTags() >> [] 1 * connectorTableServiceProxy.delete(_) >> {throw new RuntimeException("Fail to drop")} - 0 * parentChildRelSvc.drop(_) + 0 * parentChildRelSvc.drop(_, _) thrown(RuntimeException) // mock drop is not enabled and it is a child table @@ -523,7 +561,7 @@ class TableServiceImplSpec extends Specification { 0 * parentChildRelSvc.getChildren(name) 1 * config.getNoTableDeleteOnTags() >> [] 0 * connectorTableServiceProxy.delete(_) - 0 * parentChildRelSvc.drop(_) + 0 * parentChildRelSvc.drop(_,_) e = thrown(RuntimeException) assert e.message.contains("is currently disabled") @@ -539,7 +577,7 @@ class TableServiceImplSpec extends Specification { 0 * parentChildRelSvc.getChildren(name) 1 * config.getNoTableDeleteOnTags() >> [] 0 * connectorTableServiceProxy.delete(_) - 0 * parentChildRelSvc.drop(_) + 0 * parentChildRelSvc.drop(_,_) e = thrown(RuntimeException) assert e.message.contains("is currently disabled") @@ -556,7 +594,7 @@ class TableServiceImplSpec extends Specification { 1 * parentChildRelSvc.getChildren(name) >> {[] as Set} 1 * config.getNoTableDeleteOnTags() >> [] 1 * connectorTableServiceProxy.delete(_) - 1 * parentChildRelSvc.drop(_) + 1 * parentChildRelSvc.drop(name, Optional.empty()) 1 * config.canDeleteTableDefinitionMetadata() >> true noExceptionThrown() @@ -572,7 +610,7 @@ class TableServiceImplSpec extends Specification { 1 * parentChildRelSvc.getChildren(name) >> {[] as Set} 1 * config.getNoTableDeleteOnTags() >> [] 1 * connectorTableServiceProxy.delete(_) - 1 * parentChildRelSvc.drop(_) + 1 * parentChildRelSvc.drop(name, Optional.empty()) 1 * config.canDeleteTableDefinitionMetadata() >> true noExceptionThrown() } diff --git a/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java b/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java index d5226244..cc021fc9 100644 --- a/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java +++ b/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java @@ -11,6 +11,8 @@ import com.netflix.metacat.common.server.usermetadata.ParentChildRelServiceException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; import org.springframework.jdbc.core.PreparedStatementSetter; import org.springframework.jdbc.core.ResultSetExtractor; import org.springframework.transaction.annotation.Isolation; @@ -21,6 +23,7 @@ import java.sql.PreparedStatement; import java.util.Map; import java.util.Set; +import java.util.Optional; import java.util.List; import java.util.ArrayList; import java.util.HashSet; @@ -45,11 +48,6 @@ public class MySqlParentChildRelMetaDataService implements ParentChildRelMetadat "DELETE FROM parent_child_relation " + "WHERE parent = ? AND parent_uuid = ? AND child = ? AND child_uuid = ? AND relation_type = ?"; - static final String SQL_RENAME_PARENT_ENTITY = "UPDATE parent_child_relation " - + "SET parent = ? WHERE parent = ?"; - static final String SQL_RENAME_CHILD_ENTITY = "UPDATE parent_child_relation " - + "SET child = ? WHERE child = ?"; - static final String SQL_DROP_CHILD = "DELETE FROM parent_child_relation " + "WHERE child = ? "; static final String SQL_DROP_PARENT = "DELETE FROM parent_child_relation " @@ -60,9 +58,16 @@ public class MySqlParentChildRelMetaDataService implements ParentChildRelMetadat static final String SQL_GET_CHILDREN = "SELECT child, child_uuid, relation_type " + "FROM parent_child_relation WHERE parent = ?"; - - static final String SQL_IS_PARENT_TABLE = "SELECT 1 FROM parent_child_relation WHERE parent = ? LIMIT 1"; - static final String SQL_IS_CHILD_TABLE = "SELECT 1 FROM parent_child_relation WHERE child = ? LIMIT 1"; + static final String SQL_RENAME_SOFT_PARENT_INSERT = + "INSERT INTO parent_child_relation (parent, parent_uuid, child, child_uuid, relation_type) " + + "SELECT ?, parent_uuid, child, child_uuid, relation_type " + + "FROM parent_child_relation " + + "WHERE parent = ?"; + static final String SQL_RENAME_SOFT_CHILD_INSERT = + "INSERT INTO parent_child_relation (parent, parent_uuid, child, child_uuid, relation_type) " + + "SELECT parent, parent_uuid, ?, child_uuid, relation_type " + + "FROM parent_child_relation " + + "WHERE child = ?"; static final String SQL_GET_PARENT_UUIDS = "SELECT DISTINCT parent_uuid FROM parent_child_relation " + "where parent = ?"; @@ -70,6 +75,9 @@ public class MySqlParentChildRelMetaDataService implements ParentChildRelMetadat static final String SQL_GET_CHILDREN_UUIDS = "SELECT DISTINCT child_uuid FROM parent_child_relation " + "where child = ?"; + static final String SQL_IS_PARENT_TABLE = "SELECT 1 FROM parent_child_relation WHERE parent = ? LIMIT 1"; + static final String SQL_IS_CHILD_TABLE = "SELECT 1 FROM parent_child_relation WHERE child = ? LIMIT 1"; + static final String SQL_GET_CHILDREN_SIZE_PER_REL = "SELECT COUNT(*) FROM parent_child_relation " + "where parent = ? and relation_type = ?"; @@ -263,7 +271,7 @@ public void deleteParentChildRelation(final QualifiedName parentName, @Override @Transactional(isolation = Isolation.SERIALIZABLE) - public void rename(final QualifiedName oldName, final QualifiedName newName) { + public Pair, Set> renameSoftInsert(final QualifiedName oldName, final QualifiedName newName) { if (isChildTable(newName)) { throw new ParentChildRelServiceException(newName + " is already a child table"); } @@ -271,72 +279,103 @@ public void rename(final QualifiedName oldName, final QualifiedName newName) { throw new ParentChildRelServiceException(newName + " is already a parent table"); } - renameParent(oldName, newName); - renameChild(oldName, newName); - log.info("Successfully rename parent child relationship for oldName={}, newName={}", - oldName, newName - ); + final Set parentUuids = renameSoftInsertParent(oldName, newName); + final Set childUuids = renameSoftInsertChild(oldName, newName); + + if (!parentUuids.isEmpty() || !childUuids.isEmpty()) { + log.info("Successfully rename parent child relationship for oldName={}, newName={}", + oldName, newName + ); + } + return new ImmutablePair<>(parentUuids, childUuids); } - private void renameParent(final QualifiedName oldName, final QualifiedName newName) { + private Set renameSoftInsertParent(final QualifiedName oldName, final QualifiedName newName) { try { jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_PARENT_ENTITY); + final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_SOFT_PARENT_INSERT); ps.setString(1, newName.toString()); ps.setString(2, oldName.toString()); return ps; }); + return getParentUuidsForParent(newName.toString()); } catch (Exception e) { throw new ParentChildRelServiceException("Fail to rename parent from oldName:" + oldName + " to " + newName + " in MySqlParentChildRelMetadataService", e); } } - private void renameChild(final QualifiedName oldName, final QualifiedName newName) { + private Set renameSoftInsertChild(final QualifiedName oldName, final QualifiedName newName) { try { jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_CHILD_ENTITY); + final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_SOFT_CHILD_INSERT); ps.setString(1, newName.toString()); ps.setString(2, oldName.toString()); return ps; }); + return getChildUuidsForChild(newName.toString()); } catch (Exception e) { throw new ParentChildRelServiceException("Fail to rename child from oldName:" + oldName + " to " + newName + " in MySqlParentChildRelMetadataService", e); } } + // Note this api is used for 2 cases in TableServiceImpl + // 1. dropTable -> it will pass an empty optional, which will delete every records associated with the name + // 2. renameTable -> it will pass a non-empty optional, which will delete every records associated with the + // name + the uuids @Override - public void drop(final QualifiedName name) { - dropParent(name); - dropChild(name); - log.info("Successfully drop parent child relationship for name={}", name); - } + public void drop(final QualifiedName name, final Optional, Set>> uuids) { + Optional> parentUUIDs = Optional.empty(); + Optional> childUUIDs = Optional.empty(); - private void dropParent(final QualifiedName name) { - try { - jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_DROP_PARENT); - ps.setString(1, name.toString()); - return ps; - }); - } catch (Exception e) { - throw new ParentChildRelServiceException("Fail to drop parent:" + name - + " in MySqlParentChildRelMetadataService", e); + if (uuids.isPresent()) { + parentUUIDs = Optional.of(uuids.get().getLeft()); + childUUIDs = Optional.of(uuids.get().getRight()); + } + + if (drop(name, parentUUIDs, SQL_DROP_PARENT) > 0 || drop(name, childUUIDs, SQL_DROP_CHILD) > 0) { + log.info("Successfully drop parent child relationship for name={} and parentUUIDs = {} and childUUIDs = {}", + name, parentUUIDs, childUUIDs); } } - private void dropChild(final QualifiedName name) { + private int drop(final QualifiedName name, final Optional> uuids, final String sql) { + final String sqlWithInClause; + final String uuidColumn = sql.equals(SQL_DROP_PARENT) ? "parent_uuid" : "child_uuid"; + + if (uuids.isPresent() && !uuids.get().isEmpty()) { + final String inClause = uuids.get().stream() + .map(uuid -> "?") + .collect(Collectors.joining(", ")); + sqlWithInClause = sql + " AND " + uuidColumn + " IN (" + inClause + ")"; + } else { + sqlWithInClause = sql; + } + try { - jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_DROP_CHILD); + return jdbcTemplate.update(connection -> { + final PreparedStatement ps = connection.prepareStatement(sqlWithInClause); ps.setString(1, name.toString()); + + if (uuids.isPresent()) { + int i = 1; + for (String uuid : uuids.get()) { + ps.setString(++i, uuid); + } + } + return ps; }); } catch (Exception e) { - throw new ParentChildRelServiceException("Fail to drop child:" + name - + " in MySqlParentChildRelMetadataService", e); + if (sql.equals(SQL_DROP_PARENT)) { + throw new ParentChildRelServiceException("Fail to drop parent:" + name + + " in MySqlParentChildRelMetadataService", e); + } else { + throw new ParentChildRelServiceException("Fail to drop child:" + name + + " in MySqlParentChildRelMetadataService", e); + } } } @@ -431,7 +470,6 @@ private Set getChildUuidsForChild(final String childName) { return ps; }, (rs, rowNum) -> rs.getString("child_uuid"))); } - private void validateUUIDs(final String name, final Set existingUUIDs, final String inputUUID,