From 7b6aabab09ccc3aae2331314d704ab2b60beda47 Mon Sep 17 00:00:00 2001 From: Satia Herfert Date: Thu, 4 Jan 2018 08:33:35 +0100 Subject: [PATCH] Revert "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, 10 insertions(+), 54 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 c48550bf70d17..8fdc7662b218d 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,7 +59,6 @@ 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; @@ -101,7 +100,6 @@ public ExecutingQuery( this.queryParameters = queryParameters; this.transactionAnnotationData = transactionAnnotationData; this.activeLockCount = activeLockCount; - this.initialActiveLocks = activeLockCount.getAsLong(); this.threadExecutingTheQueryId = threadExecutingTheQueryId; this.threadExecutingTheQueryName = threadExecutingTheQueryName; this.cpuClock = cpuClock; @@ -156,9 +154,8 @@ 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 ); @@ -183,7 +180,7 @@ public QuerySnapshot snapshot() NANOSECONDS.toMillis( waitTimeNanos ), status.name(), status.toMap( currentTimeNanos ), - totalActiveLocks - initialActiveLocks, + activeLockCount, 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 82a5ff4b1b647..15506a06cfc53 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, - (/*activeLockCount:*/) -> 0, + null, 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 7191ab42f9bb9..3cebf72abd931 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,6 +19,11 @@ */ 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; @@ -30,11 +35,6 @@ 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,7 +62,6 @@ 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; @@ -264,39 +263,6 @@ 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 { @@ -539,7 +505,7 @@ public T resource() } } - private Resource test( Supplier setup, BiConsumer lock, String... queries ) + private Resource test( Supplier setup, BiConsumer lock, String query ) throws TimeoutException, InterruptedException, ExecutionException { CountDownLatch resourceLocked = new CountDownLatch( 1 ), listQueriesLatch = new CountDownLatch( 1 ); @@ -563,14 +529,7 @@ private Resource test( Supplier setup, BiConsumer lock, threads.executeAndAwait( parameter -> { - try ( Transaction tx = db.beginTx() ) - { - for ( String query : queries ) - { - db.execute( query ).close(); - } - tx.success(); - } + db.execute( query ).close(); return null; }, null, waitingWhileIn( GraphDatabaseFacade.class, "execute" ), SECONDS_TIMEOUT, SECONDS );