From 9a8259ef3f22abf9781513b823a9f1096152d494 Mon Sep 17 00:00:00 2001 From: Jermy Li Date: Thu, 21 Apr 2022 18:59:56 +0800 Subject: [PATCH] fix schema label: addIndexLabel/removeIndexLabel race condition (#1807) --- .../backend/serializer/BinarySerializer.java | 4 +-- .../backend/serializer/TableSerializer.java | 4 +-- .../backend/serializer/TextSerializer.java | 4 +-- .../backend/tx/SchemaTransaction.java | 36 +++++++++++++++++-- .../job/schema/IndexLabelRemoveJob.java | 5 +-- .../baidu/hugegraph/job/schema/SchemaJob.java | 22 ------------ .../baidu/hugegraph/schema/IndexLabel.java | 9 ++--- .../baidu/hugegraph/schema/SchemaLabel.java | 4 +-- .../schema/builder/IndexLabelBuilder.java | 22 ++++++------ .../baidu/hugegraph/structure/HugeIndex.java | 2 +- 10 files changed, 62 insertions(+), 50 deletions(-) diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java index a07515db98..bdfb6d2e88 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/BinarySerializer.java @@ -1051,7 +1051,7 @@ public VertexLabel readVertexLabel(HugeGraph graph, vertexLabel.properties(readIds(HugeKeys.PROPERTIES)); vertexLabel.primaryKeys(readIds(HugeKeys.PRIMARY_KEYS)); vertexLabel.nullableKeys(readIds(HugeKeys.NULLABLE_KEYS)); - vertexLabel.indexLabels(readIds(HugeKeys.INDEX_LABELS)); + vertexLabel.addIndexLabels(readIds(HugeKeys.INDEX_LABELS)); vertexLabel.enableLabelIndex(readBool(HugeKeys.ENABLE_LABEL_INDEX)); vertexLabel.status(readEnum(HugeKeys.STATUS, SchemaStatus.class)); vertexLabel.ttl(readLong(HugeKeys.TTL)); @@ -1092,7 +1092,7 @@ public EdgeLabel readEdgeLabel(HugeGraph graph, edgeLabel.properties(readIds(HugeKeys.PROPERTIES)); edgeLabel.sortKeys(readIds(HugeKeys.SORT_KEYS)); edgeLabel.nullableKeys(readIds(HugeKeys.NULLABLE_KEYS)); - edgeLabel.indexLabels(readIds(HugeKeys.INDEX_LABELS)); + edgeLabel.addIndexLabels(readIds(HugeKeys.INDEX_LABELS)); edgeLabel.enableLabelIndex(readBool(HugeKeys.ENABLE_LABEL_INDEX)); edgeLabel.status(readEnum(HugeKeys.STATUS, SchemaStatus.class)); edgeLabel.ttl(readLong(HugeKeys.TTL)); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TableSerializer.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TableSerializer.java index 0079ba5191..7a56fe50a6 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TableSerializer.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TableSerializer.java @@ -496,7 +496,7 @@ public VertexLabel readVertexLabel(HugeGraph graph, vertexLabel.properties(this.toIdArray(properties)); vertexLabel.primaryKeys(this.toIdArray(primaryKeys)); vertexLabel.nullableKeys(this.toIdArray(nullableKeys)); - vertexLabel.indexLabels(this.toIdArray(indexLabels)); + vertexLabel.addIndexLabels(this.toIdArray(indexLabels)); vertexLabel.status(status); vertexLabel.ttl(ttl.longValue()); vertexLabel.ttlStartTime(this.toId(ttlStartTime)); @@ -535,7 +535,7 @@ public EdgeLabel readEdgeLabel(HugeGraph graph, BackendEntry backendEntry) { edgeLabel.properties(this.toIdArray(properties)); edgeLabel.sortKeys(this.toIdArray(sortKeys)); edgeLabel.nullableKeys(this.toIdArray(nullableKeys)); - edgeLabel.indexLabels(this.toIdArray(indexLabels)); + edgeLabel.addIndexLabels(this.toIdArray(indexLabels)); edgeLabel.status(status); edgeLabel.ttl(ttl.longValue()); edgeLabel.ttlStartTime(this.toId(ttlStartTime)); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java index b90646b6fd..68f130a9aa 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/serializer/TextSerializer.java @@ -586,7 +586,7 @@ public VertexLabel readVertexLabel(HugeGraph graph, vertexLabel.properties(readIds(properties)); vertexLabel.primaryKeys(readIds(primaryKeys)); vertexLabel.nullableKeys(readIds(nullableKeys)); - vertexLabel.indexLabels(readIds(indexLabels)); + vertexLabel.addIndexLabels(readIds(indexLabels)); vertexLabel.enableLabelIndex(JsonUtil.fromJson(enableLabelIndex, Boolean.class)); readUserdata(vertexLabel, entry); @@ -648,7 +648,7 @@ public EdgeLabel readEdgeLabel(HugeGraph graph, edgeLabel.properties(readIds(properties)); edgeLabel.sortKeys(readIds(sortKeys)); edgeLabel.nullableKeys(readIds(nullablekeys)); - edgeLabel.indexLabels(readIds(indexLabels)); + edgeLabel.addIndexLabels(readIds(indexLabels)); edgeLabel.enableLabelIndex(JsonUtil.fromJson(enableLabelIndex, Boolean.class)); readUserdata(edgeLabel, entry); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/SchemaTransaction.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/SchemaTransaction.java index af567b384f..7563dbad04 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/SchemaTransaction.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/SchemaTransaction.java @@ -242,8 +242,40 @@ public void addIndexLabel(SchemaLabel schemaLabel, IndexLabel indexLabel) { if (schemaLabel.equals(VertexLabel.OLAP_VL)) { return; } - schemaLabel.indexLabel(indexLabel.id()); - this.updateSchema(schemaLabel); + + // FIXME: move schemaLabel update into updateSchema() lock block instead + synchronized (schemaLabel) { + schemaLabel.addIndexLabel(indexLabel.id()); + this.updateSchema(schemaLabel); + } + } + + @Watched(prefix = "schema") + public void removeIndexLabelFromBaseLabel(IndexLabel indexLabel) { + HugeType baseType = indexLabel.baseType(); + Id baseValue = indexLabel.baseValue(); + SchemaLabel schemaLabel; + if (baseType == HugeType.VERTEX_LABEL) { + schemaLabel = this.getVertexLabel(baseValue); + } else { + assert baseType == HugeType.EDGE_LABEL; + schemaLabel = this.getEdgeLabel(baseValue); + } + + if (schemaLabel == null) { + LOG.info("The base label '{}' of index label '{}' " + + "may be deleted before", baseValue, indexLabel); + return; + } + if (schemaLabel.equals(VertexLabel.OLAP_VL)) { + return; + } + + // FIXME: move schemaLabel update into updateSchema() lock block instead + synchronized (schemaLabel) { + schemaLabel.removeIndexLabel(indexLabel.id()); + this.updateSchema(schemaLabel); + } } @Watched(prefix = "schema") diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/IndexLabelRemoveJob.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/IndexLabelRemoveJob.java index d81e46022b..085b72aa1b 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/IndexLabelRemoveJob.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/IndexLabelRemoveJob.java @@ -60,8 +60,9 @@ protected static void removeIndexLabel(HugeGraphParams graph, Id id) { // Set index label to "deleting" status schemaTx.updateSchemaStatus(indexLabel, SchemaStatus.DELETING); try { - // Remove label from indexLabels of vertex or edge label - removeIndexLabelFromBaseLabel(schemaTx, indexLabel); + // Remove indexLabel from indexLabels of vertex/edge label + schemaTx.removeIndexLabelFromBaseLabel(indexLabel); + // Remove index data // TODO: use event to replace direct call graphTx.removeIndex(indexLabel); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/SchemaJob.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/SchemaJob.java index ed76824335..cb37841335 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/SchemaJob.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/job/schema/SchemaJob.java @@ -9,10 +9,7 @@ import com.baidu.hugegraph.backend.id.IdGenerator; import com.baidu.hugegraph.backend.tx.SchemaTransaction; import com.baidu.hugegraph.job.SysJob; -import com.baidu.hugegraph.schema.IndexLabel; import com.baidu.hugegraph.schema.SchemaElement; -import com.baidu.hugegraph.schema.SchemaLabel; -import com.baidu.hugegraph.schema.VertexLabel; import com.baidu.hugegraph.type.HugeType; import com.baidu.hugegraph.util.E; import com.baidu.hugegraph.util.Log; @@ -65,25 +62,6 @@ public static String formatTaskName(HugeType type, Id id, String name) { return String.join(SPLITOR, type.toString(), id.asString(), name); } - protected static void removeIndexLabelFromBaseLabel(SchemaTransaction tx, - IndexLabel label) { - HugeType baseType = label.baseType(); - Id baseValue = label.baseValue(); - SchemaLabel schemaLabel; - if (baseType == HugeType.VERTEX_LABEL) { - if (VertexLabel.OLAP_VL.id().equals(baseValue)) { - return; - } - schemaLabel = tx.getVertexLabel(baseValue); - } else { - assert baseType == HugeType.EDGE_LABEL; - schemaLabel = tx.getEdgeLabel(baseValue); - } - assert schemaLabel != null; - schemaLabel.removeIndexLabel(label.id()); - updateSchema(tx, schemaLabel); - } - /** * Use reflection to call SchemaTransaction.removeSchema(), * which is protected diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/IndexLabel.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/IndexLabel.java index 6bf2457470..64c9723e20 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/IndexLabel.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/IndexLabel.java @@ -118,8 +118,8 @@ public Id indexField() { return this.indexFields.get(0); } - public SchemaLabel baseElement() { - return getElement(this.graph, this.baseType, this.baseValue); + public SchemaLabel baseLabel() { + return getBaseLabel(this.graph, this.baseType, this.baseValue); } public boolean hasSameContent(IndexLabel other) { @@ -210,8 +210,9 @@ public static IndexLabel label(HugeGraph graph, Id id) { return graph.indexLabel(id); } - public static SchemaLabel getElement(HugeGraph graph, - HugeType baseType, Object baseValue) { + public static SchemaLabel getBaseLabel(HugeGraph graph, + HugeType baseType, + Object baseValue) { E.checkNotNull(baseType, "base type", "index label"); E.checkNotNull(baseValue, "base value", "index label"); E.checkArgument(baseValue instanceof String || baseValue instanceof Id, diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/SchemaLabel.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/SchemaLabel.java index 354fb88606..ed8c1c1a86 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/SchemaLabel.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/SchemaLabel.java @@ -93,11 +93,11 @@ public Set indexLabels() { return Collections.unmodifiableSet(this.indexLabels); } - public void indexLabel(Id id) { + public void addIndexLabel(Id id) { this.indexLabels.add(id); } - public void indexLabels(Id... ids) { + public void addIndexLabels(Id... ids) { this.indexLabels.addAll(Arrays.asList(ids)); } diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/builder/IndexLabelBuilder.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/builder/IndexLabelBuilder.java index 2521b1b1a7..44b21f4bec 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/builder/IndexLabelBuilder.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/schema/builder/IndexLabelBuilder.java @@ -86,8 +86,9 @@ public IndexLabelBuilder(SchemaTransaction transaction, super(transaction, graph); E.checkNotNull(copy, "copy"); // Get base element from self graph - SchemaLabel schemaLabel = IndexLabel.getElement(graph, copy.baseType(), - copy.baseValue()); + SchemaLabel schemaLabel = IndexLabel.getBaseLabel(graph, + copy.baseType(), + copy.baseValue()); this.id = null; this.name = copy.name(); this.baseType = copy.baseType(); @@ -110,7 +111,7 @@ public IndexLabel build() { this.checkFields4Range(); IndexLabel indexLabel = new IndexLabel(graph, id, this.name); indexLabel.baseType(this.baseType); - SchemaLabel schemaLabel = this.loadElement(); + SchemaLabel schemaLabel = this.loadBaseLabel(); indexLabel.baseValue(schemaLabel.id()); indexLabel.indexType(this.indexType); for (String field : this.indexFields) { @@ -137,7 +138,7 @@ private boolean hasSameProperties(IndexLabel existedIndexLabel) { return false; } - SchemaLabel schemaLabel = this.loadElement(); + SchemaLabel schemaLabel = this.loadBaseLabel(); if (!schemaLabel.id().equals(existedIndexLabel.baseValue())) { return false; } @@ -200,7 +201,7 @@ public SchemaElement.TaskWithSchema createWithTask() { IdGenerator.ZERO); } - SchemaLabel schemaLabel = this.loadElement(); + SchemaLabel schemaLabel = this.loadBaseLabel(); /* * If new index label is prefix of existed index label, or has @@ -286,7 +287,7 @@ public IndexLabel append() { this.checkStableVars(); Userdata.check(this.userdata, Action.APPEND); indexLabel.userdata(this.userdata); - SchemaLabel schemaLabel = indexLabel.baseElement(); + SchemaLabel schemaLabel = indexLabel.baseLabel(); this.graph().addIndexLabel(schemaLabel, indexLabel); return indexLabel; } @@ -302,7 +303,7 @@ public IndexLabel eliminate() { Userdata.check(this.userdata, Action.ELIMINATE); indexLabel.removeUserdata(this.userdata); - SchemaLabel schemaLabel = indexLabel.baseElement(); + SchemaLabel schemaLabel = indexLabel.baseLabel(); this.graph().addIndexLabel(schemaLabel, indexLabel); return indexLabel; } @@ -454,9 +455,9 @@ private void checkIndexType() { } } - private SchemaLabel loadElement() { - return IndexLabel.getElement(this.graph(), - this.baseType, this.baseValue); + private SchemaLabel loadBaseLabel() { + return IndexLabel.getBaseLabel(this.graph(), + this.baseType, this.baseValue); } private void checkFields(Set propertyIds) { @@ -619,7 +620,6 @@ private Set removeSubIndex(SchemaLabel schemaLabel) { } Set tasks = InsertionOrderUtil.newSet(); for (Id id : overrideIndexLabelIds) { - schemaLabel.removeIndexLabel(id); Id task = this.graph().removeIndexLabel(id); E.checkNotNull(task, "remove sub index label task"); tasks.add(task); diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/structure/HugeIndex.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/structure/HugeIndex.java index 9dedfe9245..fec8a0a946 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/structure/HugeIndex.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/structure/HugeIndex.java @@ -151,7 +151,7 @@ public boolean hasTtl() { if (this.indexLabel.system()) { return false; } - return this.indexLabel.baseElement().ttl() > 0L; + return this.indexLabel.baseLabel().ttl() > 0L; } public long ttl() {