diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/HugeGraph.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/HugeGraph.java index 7143cc452e..c7c43edf21 100644 --- a/hugegraph-core/src/main/java/com/baidu/hugegraph/HugeGraph.java +++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/HugeGraph.java @@ -470,7 +470,8 @@ public void close() throws HugeException { } // Make sure that all transactions are closed in all threads E.checkState(this.tx.closed(), - "Ensure tx closed in all threads when closing graph"); + "Ensure tx closed in all threads when closing graph '%s'", + this.name); } public void closeTx() { @@ -590,27 +591,25 @@ public static void shutdown(long timeout) { private class TinkerpopTransaction extends AbstractThreadLocalTransaction { - private AtomicInteger refs; - - private ThreadLocal opened; - + // Times opened from upper layer + private final AtomicInteger refs; + // Flag opened of each thread + private final ThreadLocal opened; // Backend transactions - private ThreadLocal graphTransaction; - private ThreadLocal schemaTransaction; + private final ThreadLocal transactions; public TinkerpopTransaction(Graph graph) { super(graph); - this.refs = new AtomicInteger(0); - + this.refs = new AtomicInteger(); this.opened = ThreadLocal.withInitial(() -> false); - this.graphTransaction = ThreadLocal.withInitial(() -> null); - this.schemaTransaction = ThreadLocal.withInitial(() -> null); + this.transactions = ThreadLocal.withInitial(() -> null); } public boolean closed() { - assert this.refs.get() >= 0 : this.refs.get(); - return this.refs.get() == 0; + int refs = this.refs.get(); + assert refs >= 0 : refs; + return refs == 0; } /** @@ -653,29 +652,20 @@ public boolean isOpen() { @Override protected void doOpen() { - this.schemaTransaction(); - this.graphTransaction(); - + this.getOrNewTransaction(); this.setOpened(); } @Override protected void doCommit() { this.verifyOpened(); - - this.schemaTransaction().commit(); - this.graphTransaction().commit(); + this.getOrNewTransaction().commit(); } @Override protected void doRollback() { this.verifyOpened(); - - try { - this.graphTransaction().rollback(); - } finally { - this.schemaTransaction().rollback(); - } + this.getOrNewTransaction().rollback(); } @Override @@ -683,7 +673,7 @@ protected void doClose() { this.verifyOpened(); try { - // Calling super will clear listeners + // Calling super.doClose() will clear listeners super.doClose(); } finally { this.resetState(); @@ -692,11 +682,8 @@ protected void doClose() { @Override public String toString() { - return String.format("TinkerpopTransaction{opened=%s, " + - "graphTx=%s, schemaTx=%s}", - this.opened.get(), - this.graphTransaction.get(), - this.schemaTransaction.get()); + return String.format("TinkerpopTransaction{opened=%s, txs=%s}", + this.opened.get(), this.transactions.get()); } private void verifyOpened() { @@ -727,33 +714,27 @@ private void setClosed() { } private SchemaTransaction schemaTransaction() { - /* - * NOTE: this method may be called even tx is not opened, - * the reason is for reusing backend tx. - * so we don't call this.verifyOpened() here. - */ - - SchemaTransaction schemaTx = this.schemaTransaction.get(); - if (schemaTx == null) { - schemaTx = openSchemaTransaction(); - this.schemaTransaction.set(schemaTx); - } - return schemaTx; + return this.getOrNewTransaction().schemaTx; } private GraphTransaction graphTransaction() { + return this.getOrNewTransaction().graphTx; + } + + private Txs getOrNewTransaction() { /* * NOTE: this method may be called even tx is not opened, * the reason is for reusing backend tx. * so we don't call this.verifyOpened() here. */ - GraphTransaction graphTx = this.graphTransaction.get(); - if (graphTx == null) { - graphTx = openGraphTransaction(); - this.graphTransaction.set(graphTx); + Txs txs = this.transactions.get(); + if (txs == null) { + // TODO: close SchemaTransaction if GraphTransaction is error + txs = new Txs(openSchemaTransaction(), openGraphTransaction()); + this.transactions.set(txs); } - return graphTx; + return txs; } private void destroyTransaction() { @@ -762,26 +743,57 @@ private void destroyTransaction() { "Transaction should be closed before destroying"); } - GraphTransaction graphTx = this.graphTransaction.get(); - if (graphTx != null) { - try { - graphTx.close(); - } catch (Exception e) { - LOG.error("Failed to close GraphTransaction", e); - } + // Do close if needed, then remove the reference + Txs txs = this.transactions.get(); + if (txs != null) { + txs.close(); + } + this.transactions.remove(); + } + } + + private static final class Txs { + + public final SchemaTransaction schemaTx; + public final GraphTransaction graphTx; + + public Txs(SchemaTransaction schemaTx, GraphTransaction graphTx) { + assert schemaTx != null && graphTx != null; + this.schemaTx = schemaTx; + this.graphTx = graphTx; + } + + public void commit() { + this.schemaTx.commit(); + this.graphTx.commit(); + } + + public void rollback() { + try { + this.schemaTx.rollback(); + } finally { + this.graphTx.rollback(); + } + } + + public void close() { + try { + this.graphTx.close(); + } catch (Exception e) { + LOG.error("Failed to close GraphTransaction", e); } - SchemaTransaction schemaTx = this.schemaTransaction.get(); - if (schemaTx != null) { - try { - schemaTx.close(); - } catch (Exception e) { - LOG.error("Failed to close SchemaTransaction", e); - } + try { + this.schemaTx.close(); + } catch (Exception e) { + LOG.error("Failed to close SchemaTransaction", e); } + } - this.graphTransaction.remove(); - this.schemaTransaction.remove(); + @Override + public String toString() { + return String.format("{schemaTx=%s,graphTx=%s}", + this.schemaTx, this.graphTx); } } } diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessBasicSuite.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessBasicSuite.java index 62f7f88908..aa09a293d8 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessBasicSuite.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessBasicSuite.java @@ -287,7 +287,7 @@ protected Statement withAfterClasses(final Statement statement) { public void evaluate() throws Throwable { statement.evaluate(); GraphProvider gp = GraphManager.setGraphProvider(null); - ((TestGraphProvider) gp).clearBackends(); + ((TestGraphProvider) gp).clear(); GraphManager.setGraphProvider(gp); } }; diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessPerformanceSuite.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessPerformanceSuite.java index ac71eb6fa9..a1433bb4a5 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessPerformanceSuite.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/ProcessPerformanceSuite.java @@ -74,7 +74,7 @@ protected Statement withAfterClasses(final Statement statement) { public void evaluate() throws Throwable { statement.evaluate(); GraphProvider gp = GraphManager.setGraphProvider(null); - ((TestGraphProvider) gp).clearBackends(); + ((TestGraphProvider) gp).clear(); GraphManager.setGraphProvider(gp); } }; diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructureBasicSuite.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructureBasicSuite.java index 782ad36372..665c298d3d 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructureBasicSuite.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructureBasicSuite.java @@ -118,7 +118,7 @@ protected Statement withAfterClasses(final Statement statement) { public void evaluate() throws Throwable { statement.evaluate(); GraphProvider gp = GraphManager.setGraphProvider(null); - ((TestGraphProvider) gp).clearBackends(); + ((TestGraphProvider) gp).clear(); GraphManager.setGraphProvider(gp); } }; diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructurePerformanceSuite.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructurePerformanceSuite.java index bef00dd6b7..2cafd56184 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructurePerformanceSuite.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/StructurePerformanceSuite.java @@ -76,7 +76,7 @@ protected Statement withAfterClasses(final Statement statement) { public void evaluate() throws Throwable { statement.evaluate(); GraphProvider gp = GraphManager.setGraphProvider(null); - ((TestGraphProvider) gp).clearBackends(); + ((TestGraphProvider) gp).clear(); GraphManager.setGraphProvider(gp); } }; diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphFactory.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphFactory.java index 6c0d8343cf..7c4012976f 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphFactory.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphFactory.java @@ -19,10 +19,12 @@ package com.baidu.hugegraph.tinkerpop; -import com.baidu.hugegraph.HugeFactory; import org.apache.commons.configuration.Configuration; +import com.baidu.hugegraph.HugeFactory; + public class TestGraphFactory { + public static TestGraph open(Configuration config) { return new TestGraph(HugeFactory.open(config)); } diff --git a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphProvider.java b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphProvider.java index 6b5ed2421c..9cb8332059 100644 --- a/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphProvider.java +++ b/hugegraph-test/src/main/java/com/baidu/hugegraph/tinkerpop/TestGraphProvider.java @@ -326,10 +326,13 @@ public Graph openTestGraph(final Configuration config) { @Override public void clear(Graph graph, Configuration config) throws Exception { TestGraph testGraph = (TestGraph) graph; - if (testGraph == null || !testGraph.initedBackend()) { + if (testGraph == null) { return; } String graphName = config.getString(CoreOptions.STORE.name()); + if (!testGraph.initedBackend()) { + testGraph.close(); + } if (testGraph.closed()) { if (this.graphs.get(graphName) == testGraph) { this.graphs.remove(graphName); @@ -351,10 +354,18 @@ public void clear(Graph graph, Configuration config) throws Exception { LOG.debug("Clear graph '{}'", graphName); } - public void clearBackends() { + public void clear() { for (TestGraph graph : this.graphs.values()) { graph.clearBackend(); } + for (TestGraph graph : this.graphs.values()) { + try { + graph.close(); + } catch (Exception e) { + LOG.error("Error while closing graph '{}'", graph, e); + } + } + this.graphs.clear(); } @Watched