Skip to content

Commit

Permalink
XD-1015 fix that adding a new link type might cause a concurrency exc…
Browse files Browse the repository at this point in the history
…eption
  • Loading branch information
kirillvasilenko committed Sep 26, 2024
1 parent a07bddb commit a5c818f
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ package jetbrains.exodus.query.metadata

import com.orientechnologies.orient.core.db.ODatabaseSession
import com.orientechnologies.orient.core.metadata.schema.OClass
import com.orientechnologies.orient.core.record.ODirection
import com.orientechnologies.orient.core.record.OVertex
import jetbrains.exodus.entitystore.orientdb.*

class OModelMetaData(
Expand Down Expand Up @@ -54,28 +52,27 @@ class OModelMetaData(
inClassName: String
): OClass {
/**
* It is not enough to check the existence of the edge class.
* It is enough to check the existence of the edge class.
* We reuse the same edge class for all the links with the same name.
* So, we have to check the existence of the edge class + in and out properties of the connected classes.
*/
// edge class
val edgeClassName = OVertexEntity.edgeClassName(linkName)
val oClass = session.getClass(edgeClassName)
// out-property
val outClass = session.getClass(outClassName) ?: throw IllegalStateException("$outClassName not found. It must not ever happen.")
val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName)
val outProp = outClass.getProperty(outPropName)
// in-property
val inClass = session.getClass(inClassName) ?: throw IllegalStateException("$inClassName not found. It must not ever happen.")
val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName)
val inProp = inClass.getProperty(inPropName)
if (oClass != null && outProp != null && inProp != null) {
if (oClass != null) {
return oClass
}

return session.executeInASeparateSessionIfCurrentHasTransaction(dbProvider) { sessionToWork ->
val link = LinkMetadata(name = linkName, outClassName = outClassName, inClassName = inClassName, AssociationEndCardinality._0_n)
val result = sessionToWork.addAssociation(link, indicesContainingLink = listOf(), applyLinkCardinality = true)

/**
* We do not apply link cardinality because:
* 1. We do not have any cardinality restrictions for ad-hoc links.
* 2. Applying the cardinality causes adding extra properties to existing vertices,
* that in turn potentially causes OConcurrentModificationException in the original session.
* Keep in mind that we create this edge class (and those extra properties) in a separate session.
* So, there is an original session with the business logic that may fail if we change vertices here.
*/
val result = sessionToWork.addAssociation(link, indicesContainingLink = listOf(), applyLinkCardinality = false)
sessionToWork.initializeIndices(result)
sessionToWork.getClass(edgeClassName) ?: throw IllegalStateException("$edgeClassName not found, it must never happen")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,33 +353,34 @@ internal class OrientDbSchemaInitializer(
appendLine()

withPadding {
// class1.prop1 -> edgeClass -> class2
applyLink(
if (applyLinkCardinality) {
applyLinkCardinality(
edgeClass,
outClass,
link.cardinality,
inClass,
)
}
applyIndices(
link.name,
edgeClass,
outClass,
link.cardinality,
inClass,
indicesContainingLink
)
}
}

private fun applyLink(
linkName: String,
private fun applyLinkCardinality(
edgeClass: OClass,
outClass: OClass,
outCardinality: AssociationEndCardinality,
inClass: OClass,
indicesContainingLink: List<Index>
) {
val linkOutPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClass.name)
append("outProp: ${outClass.name}.$linkOutPropName")
val outProp = outClass.createLinkPropertyIfAbsent(linkOutPropName)
if (applyLinkCardinality) {
// applying cardinality only to out direct property
outProp.applyCardinality(outCardinality)
}
// applying cardinality only to out direct property
outProp.applyCardinality(outCardinality)
appendLine()

val linkInPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClass.name)
Expand All @@ -391,7 +392,14 @@ internal class OrientDbSchemaInitializer(
* We do not apply cardinality for the in-properties because, we do not know if there is any restrictions.
* Because AssociationEndCardinality describes the cardinality of a single end.
* */
}

private fun applyIndices(
linkName: String,
edgeClass: OClass,
outClass: OClass,
indicesContainingLink: List<Index>
) {
addIndex(linkUniqueIndex(edgeClass.name))

if (indicesContainingLink.isNotEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,8 @@ class OModelMetaDataTest: OTestMixin {

withSession { session ->
// check that the necessary schema parts have been initialized
session.assertAssociationExists("type1", "type2", "link1", AssociationEndCardinality._0_n)
session.assertAssociationExists("type2", "type1", "link2", AssociationEndCardinality._0_n)
session.assertAssociationExists("type1", "type2", "link1", cardinality = null)
session.assertAssociationExists("type2", "type1", "link2", cardinality = null)

/**
* It is a tricky one.
Expand All @@ -309,7 +309,40 @@ class OModelMetaDataTest: OTestMixin {

// check that the necessary schema parts have been initialized
withSession { session ->
session.assertAssociationExists("type2", "type1", "link1", AssociationEndCardinality._0_n)
session.assertAssociationExists("type2", "type1", "link1", cardinality = null)
}
}

@Test
fun `adding new link type does not cause OConcurrentModificationException`() {
val model = oModel(orientDb.provider) {
entity("type1")
entity("type2")
}
// initialize the entity types
model.prepare()
val store = OPersistentEntityStore(orientDb.provider, orientDb.dbName, model)

// entity1 has already existed for a while
val id1 = store.computeInTransaction { tx ->
val e1 = tx.newEntity("type1")
e1.setProperty("trista", "opca")
e1.id
}

/**
* Initializing new links must not affect vertices.
* 1. All the business logic happens in session1.
* 1. Initializing new links happens in a separate session, session2.
* 2. Everything that happens in session2 is concurrent changes from the session1 point of view.
* 3. So, if the initialization of new links affects vertices, session1 will fail with OConcurrentModificationException.
*/
store.executeInTransaction { tx ->
val e1 = tx.getEntity(id1)
val e2 = tx.newEntity("type2")
e1.addLink("link1", e2)
e2.addLink("link2", e1)
e1.setProperty("trista", "drista")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,26 +53,28 @@ internal fun ODatabaseSession.assertAssociationExists(
outClassName: String,
inClassName: String,
edgeName: String,
cardinality: AssociationEndCardinality?
cardinality: AssociationEndCardinality?,
) {
val edgeClassName = edgeName.asEdgeClass
val edgeClass = getClass(edgeClassName)
val inClass = getClass(inClassName)!!
val outClass = getClass(outClassName)!!

val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName)
val directOutProp = outClass.getProperty(outPropName)!!
assertEquals(OType.LINKBAG, directOutProp.type)
directOutProp.assertCardinality(cardinality)
assertTrue(edgeClass.areIndexed(OEdge.DIRECTION_IN, OEdge.DIRECTION_OUT))

val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName)
val directInProp = inClass.getProperty(inPropName)!!
assertEquals(OType.LINKBAG, directInProp.type)
if (cardinality != null) {
val outPropName = OVertex.getEdgeLinkFieldName(ODirection.OUT, edgeClassName)
val directOutProp = outClass.getProperty(outPropName)!!
assertEquals(OType.LINKBAG, directOutProp.type)
directOutProp.assertCardinality(cardinality)

assertTrue(edgeClass.areIndexed(OEdge.DIRECTION_IN, OEdge.DIRECTION_OUT))
val inPropName = OVertex.getEdgeLinkFieldName(ODirection.IN, edgeClassName)
val directInProp = inClass.getProperty(inPropName)!!
assertEquals(OType.LINKBAG, directInProp.type)
}
}

private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality?) {
private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality) {
when (cardinality) {
AssociationEndCardinality._0_1 -> {
assertTrue(!this.isMandatory)
Expand All @@ -97,12 +99,6 @@ private fun OProperty.assertCardinality(cardinality: AssociationEndCardinality?)
assertTrue(this.min == "1")
assertTrue(this.max == null)
}

null -> {
assertTrue(!this.isMandatory)
assertTrue(this.min == null)
assertTrue(this.max == null)
}
}
}

Expand Down

0 comments on commit a5c818f

Please sign in to comment.