From 517ef126d803b365dd44ea256f59a18cd0a3d01f Mon Sep 17 00:00:00 2001 From: Tobias Lindaaker Date: Tue, 19 Dec 2017 19:30:37 +0100 Subject: [PATCH] Only count the locks acquired by the current query --- .../kernel/api/query/ExecutingQuery.java | 7 ++- .../api/query/ExecutingQueryStatusTest.java | 2 +- .../ListQueriesProcedureTest.java | 55 ++++++++++++++++--- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/query/ExecutingQuery.java b/community/kernel/src/main/java/org/neo4j/kernel/api/query/ExecutingQuery.java index 4c52f692fa7a4..c5c23d89a2a71 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/query/ExecutingQuery.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/query/ExecutingQuery.java @@ -59,6 +59,7 @@ public class ExecutingQuery @SuppressWarnings( {"unused", "FieldCanBeLocal"} ) private final String threadExecutingTheQueryName; private final LongSupplier activeLockCount; + private final long initialActiveLocks; private final SystemNanoClock clock; private final CpuClock cpuClock; private final HeapAllocation heapAllocation; @@ -100,6 +101,7 @@ public ExecutingQuery( this.queryParameters = queryParameters; this.transactionAnnotationData = transactionAnnotationData; this.activeLockCount = activeLockCount; + this.initialActiveLocks = activeLockCount.getAsLong(); this.threadExecutingTheQueryId = threadExecutingTheQueryId; this.threadExecutingTheQueryName = threadExecutingTheQueryName; this.cpuClock = cpuClock; @@ -154,8 +156,9 @@ public QuerySnapshot snapshot() long planningDoneNanos = this.planningDoneNanos; // guarded by barrier - like planningDoneNanos PlannerInfo planner = status.isPlanning() ? null : this.plannerInfo; + // activeLockCount is not atomic to capture, so we capture it after the most sensitive part. + long totalActiveLocks = this.activeLockCount.getAsLong(); // just needs to be captured at some point... - long activeLockCount = this.activeLockCount.getAsLong(); long heapAllocatedBytes = heapAllocation.allocatedBytes( threadExecutingTheQueryId ); PageCounterValues pageCounters = new PageCounterValues( pageCursorCounters ); @@ -180,7 +183,7 @@ public QuerySnapshot snapshot() NANOSECONDS.toMillis( waitTimeNanos ), status.name(), status.toMap( currentTimeNanos ), - activeLockCount, + totalActiveLocks - initialActiveLocks, heapAllocatedBytes ); } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/query/ExecutingQueryStatusTest.java b/community/kernel/src/test/java/org/neo4j/kernel/api/query/ExecutingQueryStatusTest.java index 4964a521277a7..1ba1d3d72c4f3 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/api/query/ExecutingQueryStatusTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/api/query/ExecutingQueryStatusTest.java @@ -100,7 +100,7 @@ public void shouldProduceSensibleMapRepresentationInWaitingOnQueryState() throws null, null, null, - null, + (/*activeLockCount:*/) -> 0, PageCursorTracer.NULL, Thread.currentThread().getId(), Thread.currentThread().getName(), diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java index dd4c8d06f540c..891d94004532a 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java +++ b/enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java @@ -19,11 +19,6 @@ */ package org.neo4j.kernel.enterprise.builtinprocs; -import org.hamcrest.Matcher; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; - import java.io.PrintWriter; import java.util.HashSet; import java.util.List; @@ -35,6 +30,11 @@ import java.util.function.BiConsumer; import java.util.function.Supplier; +import org.hamcrest.Matcher; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; + import org.neo4j.graphdb.Node; import org.neo4j.graphdb.Result; import org.neo4j.graphdb.Transaction; @@ -62,6 +62,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.neo4j.graphdb.Label.label; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.cypher_hints_error; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.track_query_allocation; @@ -263,6 +264,39 @@ public void shouldListActiveLocks() throws Exception } } + @Test + public void shouldOnlyGetActiveLockCountFromCurrentQuery() throws Exception + { + // given + String query1 = "MATCH (x:X) SET x.v = 1"; + String query2 = "MATCH (y:Y) SET y.v = 2 WITH count(y) AS y MATCH (z:Z) SET z.v = y"; + try ( Resource test = test( () -> + { + for ( int i = 0; i < 5; i++ ) + { + db.createNode( label( "X" ) ); + } + db.createNode( label( "Y" ) ); + return db.createNode( label( "Z" ) ); + }, Transaction::acquireWriteLock, query1, query2 ) ) + { + // when + try ( Result rows = db.execute( "CALL dbms.listQueries() " + + "YIELD query AS queryText, queryId, activeLockCount " + + "WHERE queryText = $queryText " + + "RETURN *", singletonMap( "queryText", query2 ) ) ) + { + assertTrue( "should have at least one row", rows.hasNext() ); + Map row = rows.next(); + Object activeLockCount = row.get( "activeLockCount" ); + assertFalse( "should have at most one row", rows.hasNext() ); + assertThat( "activeLockCount", activeLockCount, instanceOf( Long.class ) ); + long lockCount = (Long) activeLockCount; + assertEquals( 1, lockCount ); + } + } + } + @Test public void shouldContainSpecificConnectionDetails() throws Exception { @@ -505,7 +539,7 @@ public T resource() } } - private Resource test( Supplier setup, BiConsumer lock, String query ) + private Resource test( Supplier setup, BiConsumer lock, String... queries ) throws TimeoutException, InterruptedException, ExecutionException { CountDownLatch resourceLocked = new CountDownLatch( 1 ), listQueriesLatch = new CountDownLatch( 1 ); @@ -529,7 +563,14 @@ private Resource test( Supplier setup, BiConsumer lock, threads.executeAndAwait( parameter -> { - db.execute( query ).close(); + try ( Transaction tx = db.beginTx() ) + { + for ( String query : queries ) + { + db.execute( query ).close(); + } + tx.success(); + } return null; }, null, waitingWhileIn( GraphDatabaseFacade.class, "execute" ), SECONDS_TIMEOUT, SECONDS );