Use commands for all counts updates

There was a issue with recovery where recovering counts would need to
read the labels of a node as it existed before the command was applied.
Since this data is not written to the log the counts recovery would read
from the store which could lead to inconsistencies as well as failure to
recover.

This is resolved by making sure that all counts updates are handled with
explicit commands, and never inferred from other commands. This
simplifies the counts update code, and ensures that counts deltas need
only be computed once (when the transaction is created).

community/consistency-check/src/test/java/org/neo4j/consistency/checking/GraphStoreFixture.java

@@ -36,20 +36,21 @@
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.kernel.GraphDatabaseAPI;
 import org.neo4j.kernel.KernelHealth;
+import org.neo4j.kernel.api.ReadOperations;
 import org.neo4j.kernel.api.direct.DirectStoreAccess;
 import org.neo4j.kernel.api.exceptions.TransactionFailureException;
 import org.neo4j.kernel.api.impl.index.DirectoryFactory;
 import org.neo4j.kernel.api.impl.index.LuceneSchemaIndexProvider;
 import org.neo4j.kernel.api.index.SchemaIndexProvider;
 import org.neo4j.kernel.configuration.Config;
-import org.neo4j.kernel.impl.api.CountsAccessor;
 import org.neo4j.kernel.impl.api.TransactionApplicationMode;
 import org.neo4j.kernel.impl.api.TransactionRepresentationCommitProcess;
 import org.neo4j.kernel.impl.api.TransactionRepresentationStoreApplier;
 import org.neo4j.kernel.impl.locking.LockGroup;
 import org.neo4j.kernel.impl.store.NeoStore;
+import org.neo4j.kernel.impl.store.NodeLabelsField;
+import org.neo4j.kernel.impl.store.NodeStore;
 import org.neo4j.kernel.impl.store.StoreAccess;
-import org.neo4j.kernel.impl.store.counts.CountsTracker;
 import org.neo4j.kernel.impl.store.record.DynamicRecord;
 import org.neo4j.kernel.impl.store.record.NeoStoreRecord;
 import org.neo4j.kernel.impl.store.record.NodeRecord;
@@ -118,13 +119,10 @@ public static abstract class Transaction
         protected abstract void transactionData( TransactionDataBuilder tx, IdGenerator next );
 
         public TransactionRepresentation representation( IdGenerator idGenerator, int masterId, int authorId,
-                                                         long lastCommittedTx, CountsTracker counts )
+                                                         long lastCommittedTx, NodeStore nodes )
         {
             TransactionWriter writer = new TransactionWriter();
-            try ( CountsAccessor.Updater updater = counts.apply( lastCommittedTx + 1 ).get() )
-            {
-                transactionData( new TransactionDataBuilder( writer, updater ), idGenerator );
-            }
+            transactionData( new TransactionDataBuilder( writer, nodes ), idGenerator );
             return writer.representation( new byte[0], masterId, authorId, startTimestamp, lastCommittedTx,
                    currentTimeMillis() );
         }
@@ -196,12 +194,12 @@ public int propertyKey()
     public static final class TransactionDataBuilder
     {
         private final TransactionWriter writer;
-        private final CountsAccessor.Updater counts;
+        private final NodeStore nodes;
 
-        public TransactionDataBuilder( TransactionWriter writer, CountsAccessor.Updater counts )
+        public TransactionDataBuilder( TransactionWriter writer, NodeStore nodes )
         {
             this.writer = writer;
-            this.counts = counts;
+            this.nodes = nodes;
         }
 
         public void createSchema( Collection<DynamicRecord> beforeRecords, Collection<DynamicRecord> afterRecords,
@@ -215,36 +213,40 @@ public void createSchema( Collection<DynamicRecord> beforeRecords, Collection<Dy
 
         public void propertyKey( int id, String key )
         {
-            writer.propertyKey( id, key, id+1 );
+            writer.propertyKey( id, key, id + 1 );
         }
 
         public void nodeLabel( int id, String name )
         {
-            writer.label( id, name, id+1 );
+            writer.label( id, name, id + 1 );
         }
 
         public void relationshipType( int id, String relationshipType )
         {
-            writer.relationshipType( id, relationshipType, id+1 );
+            writer.relationshipType( id, relationshipType, id + 1 );
         }
 
-        public void create( NodeRecord node )
+        public void update( NeoStoreRecord record )
         {
-            writer.create( node );
+            writer.update( record );
         }
 
-        public void update( NeoStoreRecord record )
+        public void create( NodeRecord node )
         {
-            writer.update( record );
+            updateCounts( node, 1 );
+            writer.create( node );
         }
 
         public void update( NodeRecord before, NodeRecord after )
         {
+            updateCounts( before, -1 );
+            updateCounts( after, 1 );
             writer.update( before, after );
         }
 
         public void delete( NodeRecord node )
         {
+            updateCounts( node, -1 );
             writer.delete( node );
         }
 
@@ -293,14 +295,23 @@ public void delete( PropertyRecord before, PropertyRecord property )
             writer.delete( before, property );
         }
 
+        private void updateCounts( NodeRecord node, int delta )
+        {
+            writer.incrementNodeCount( ReadOperations.ANY_LABEL, delta );
+            for ( long label : NodeLabelsField.parseLabelsField( node ).get( nodes ) )
+            {
+                writer.incrementNodeCount( (int)label, delta );
+            }
+        }
+
         public void incrementNodeCount( int labelId, long delta )
         {
-            counts.incrementNodeCount( labelId, delta );
+            writer.incrementNodeCount( labelId, delta );
         }
 
         public void incrementRelationshipCount( int startLabelId, int typeId, int endLabelId, long delta )
         {
-            counts.incrementRelationshipCount( startLabelId, typeId, endLabelId, delta );
+            writer.incrementRelationshipCount( startLabelId, typeId, endLabelId, delta );
         }
     }
 
@@ -350,9 +361,9 @@ protected void applyTransaction( Transaction transaction ) throws TransactionFai
                             TransactionApplicationMode.EXTERNAL );
             TransactionIdStore transactionIdStore = database.getDependencyResolver().resolveDependency(
                     TransactionIdStore.class );
-            CountsTracker counts = database.getDependencyResolver().resolveDependency( NeoStore.class ).getCounts();
+            NodeStore nodes = database.getDependencyResolver().resolveDependency( NeoStore.class ).getNodeStore();
             commitProcess.commit( transaction.representation( idGenerator(), masterId(), myId(),
-                    transactionIdStore.getLastCommittedTransactionId(), counts ), locks,
+                    transactionIdStore.getLastCommittedTransactionId(), nodes ), locks,
                     CommitEvent.NULL );
         }
         finally

community/consistency-check/src/test/java/org/neo4j/consistency/checking/TransactionWriter.java

@@ -284,6 +284,16 @@ public void add( NeoStoreRecord record )
         addCommand( command );
     }
 
+    public void incrementNodeCount( int labelId, long delta )
+    {
+        addCommand( new Command.NodeCountsCommand().init( labelId, delta ) );
+    }
+
+    public void incrementRelationshipCount( int startLabelId, int typeId, int endLabelId, long delta )
+    {
+        addCommand( new Command.RelationshipCountsCommand().init( startLabelId, typeId, endLabelId, delta ) );
+    }
+
     private static <T extends TokenRecord> T withName( T record, int[] dynamicIds, String name )
     {
         if ( dynamicIds == null || dynamicIds.length == 0 )

community/consistency-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java

@@ -19,17 +19,6 @@
  */
 package org.neo4j.consistency.checking.full;
 
-import org.junit.Ignore;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TestRule;
-import org.junit.runner.Description;
-import org.junit.runner.RunWith;
-import org.junit.runners.Parameterized;
-import org.junit.runners.Parameterized.Parameter;
-import org.junit.runners.Parameterized.Parameters;
-import org.junit.runners.model.Statement;
-
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
@@ -41,6 +30,17 @@
 import java.util.List;
 import java.util.Set;
 
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+import org.junit.runners.model.Statement;
+
 import org.neo4j.consistency.RecordType;
 import org.neo4j.consistency.checking.GraphStoreFixture;
 import org.neo4j.consistency.report.ConsistencySummaryStatistics;
@@ -89,8 +89,10 @@
 import org.neo4j.unsafe.batchinsert.LabelScanWriter;
 
 import static java.util.Arrays.asList;
+
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+
 import static org.neo4j.consistency.checking.RecordCheckTestBase.inUse;
 import static org.neo4j.consistency.checking.RecordCheckTestBase.notInUse;
 import static org.neo4j.consistency.checking.full.ExecutionOrderIntegrationTest.config;
@@ -100,6 +102,8 @@
 import static org.neo4j.graphdb.DynamicRelationshipType.withName;
 import static org.neo4j.helpers.collection.IteratorUtil.asIterable;
 import static org.neo4j.helpers.collection.IteratorUtil.iterator;
+import static org.neo4j.kernel.api.ReadOperations.ANY_LABEL;
+import static org.neo4j.kernel.api.ReadOperations.ANY_RELATIONSHIP_TYPE;
 import static org.neo4j.kernel.api.labelscan.NodeLabelUpdate.labelChanges;
 import static org.neo4j.kernel.impl.store.AbstractDynamicStore.readFullByteArrayFromHeavyRecords;
 import static org.neo4j.kernel.impl.store.DynamicArrayStore.allocateFromNumbers;
@@ -677,7 +681,7 @@ protected void transactionData( GraphStoreFixture.TransactionDataBuilder tx,
 
         // then
         on( stats ).verify( RecordType.RELATIONSHIP, 2 )
-                   .verify( RecordType.COUNTS, 1 )
+                   .verify( RecordType.COUNTS, 3 )
                    .andThatsAllFolks();
     }
 
@@ -1182,6 +1186,8 @@ protected void transactionData( GraphStoreFixture.TransactionDataBuilder tx,
                 tx.create( withNext( inUse( new RelationshipRecord( relA, otherNode, otherNode, typeId ) ), relB ) );
                 tx.create( withPrev( inUse( new RelationshipRecord( relB, otherNode, otherNode, typeId ) ), relA ) );
                 tx.create( withOwner( withRelationships( inUse( new RelationshipGroupRecord( group, typeId ) ), relB, relB, relB ), node ) );
+                tx.incrementRelationshipCount( ANY_LABEL, ANY_RELATIONSHIP_TYPE, ANY_LABEL, 2 );
+                tx.incrementRelationshipCount( ANY_LABEL, typeId, ANY_LABEL, 2 );
             }
         } );
 
@@ -1356,6 +1362,8 @@ protected void transactionData( GraphStoreFixture.TransactionDataBuilder tx,
                 tx.create( new RelationshipRecord( rel, otherNode, otherNode, typeB ) );
                 tx.create( withOwner( withRelationships( new RelationshipGroupRecord( group, typeA ),
                         rel, rel, rel ), node ) );
+                tx.incrementRelationshipCount( ANY_LABEL, ANY_RELATIONSHIP_TYPE, ANY_LABEL, 1 );
+                tx.incrementRelationshipCount( ANY_LABEL, typeB, ANY_LABEL, 1 );
             }
         } );
 
@@ -1396,6 +1404,8 @@ protected void transactionData( GraphStoreFixture.TransactionDataBuilder tx,
                 tx.create( new NodeRecord( nodeA, true, groupA, NO_NEXT_PROPERTY.intValue() ) );
                 tx.create( new NodeRecord( nodeB, false, rel, NO_NEXT_PROPERTY.intValue() ) );
                 tx.create( firstInChains( new RelationshipRecord( rel, nodeA, nodeB, typeA ), 1 ) );
+                tx.incrementRelationshipCount( ANY_LABEL, ANY_RELATIONSHIP_TYPE, ANY_LABEL, 1 );
+                tx.incrementRelationshipCount( ANY_LABEL, typeA, ANY_LABEL, 1 );
 
                 tx.create( withOwner( withRelationship( withNext( new RelationshipGroupRecord( groupA, typeA ), groupB ),
                         Direction.OUTGOING, rel ), nodeA ) );

community/kernel/src/main/java/org/neo4j/kernel/impl/api/CommandApplierFacade.java

@@ -32,9 +32,11 @@
 import org.neo4j.kernel.impl.transaction.command.Command.LabelTokenCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.NeoStoreCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.NodeCommand;
+import org.neo4j.kernel.impl.transaction.command.Command.NodeCountsCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.PropertyCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.PropertyKeyTokenCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.RelationshipCommand;
+import org.neo4j.kernel.impl.transaction.command.Command.RelationshipCountsCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.RelationshipGroupCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.RelationshipTypeTokenCommand;
 import org.neo4j.kernel.impl.transaction.command.Command.SchemaRuleCommand;
@@ -292,12 +294,26 @@ public boolean visitIndexDefineCommand( IndexDefineCommand command ) throws IOEx
     }
 
     @Override
-    public boolean visitUpdateCountsCommand( Command.CountsCommand command ) throws IOException
+    public boolean visitNodeCountsCommand( NodeCountsCommand command ) throws IOException
     {
         boolean result = false;
         for ( NeoCommandHandler handler : handlers )
         {
-            if ( handler.visitUpdateCountsCommand( command ) )
+            if ( handler.visitNodeCountsCommand( command ) )
+            {
+                result = true;
+            }
+        }
+        return result;
+    }
+
+    @Override
+    public boolean visitRelationshipCountsCommand( RelationshipCountsCommand command ) throws IOException
+    {
+        boolean result = false;
+        for ( NeoCommandHandler handler : handlers )
+        {
+            if ( handler.visitRelationshipCountsCommand( command ) )
             {
                 result = true;
             }

community/kernel/src/main/java/org/neo4j/kernel/impl/api/CountsRecordState.java

@@ -78,7 +78,10 @@ public DoubleLongRegister indexSample( int labelId, int propertyKeyId, DoubleLon
     @Override
     public void incrementRelationshipCount( int startLabelId, int typeId, int endLabelId, long delta )
     {
-        counts( relationshipKey( startLabelId, typeId, endLabelId ) ).increment( 0l, delta );
+        if ( delta != 0 )
+        {
+            counts( relationshipKey( startLabelId, typeId, endLabelId ) ).increment( 0l, delta );
+        }
     }
 
     @Override
@@ -282,12 +285,21 @@ private static class CommandCollector extends CountsVisitor.Adapter
             this.commands = commands;
         }
 
+        @Override
+        public void visitNodeCount( int labelId, long count )
+        {
+            if ( count != 0 )
+            {   // Only add commands for counts that actually change
+                commands.add( new Command.NodeCountsCommand().init( labelId, count ) );
+            }
+        }
+
         @Override
         public void visitRelationshipCount( int startLabelId, int typeId, int endLabelId, long count )
         {
             if ( count != 0 )
             {   // Only add commands for counts that actually change
-                commands.add( new Command.CountsCommand().init( startLabelId, typeId, endLabelId, count ) );
+                commands.add( new Command.RelationshipCountsCommand().init( startLabelId, typeId, endLabelId, count ) );
             }
         }
     }