Unflake IndexStatisticsTest

There is an optimization in the node.setProperty that does not send an
index update if the old and now values are equal. This was not
compensated for in the test.

community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/PopulatingIndexProxy.java

@@ -66,7 +66,7 @@ public IndexUpdater newUpdater( final IndexUpdateMode mode )
                 return new PopulatingIndexUpdater()
                 {
                     @Override
-                    public void process( IndexEntryUpdate<?> update ) throws IOException, IndexEntryConflictException
+                    public void process( IndexEntryUpdate<?> update )
                     {
                         job.update( update );
                     }

community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/IndexStatisticsTest.java

@@ -72,6 +72,31 @@
 import static org.junit.runners.Parameterized.Parameter;
 import static org.junit.runners.Parameterized.Parameters;
 
+/**
+ * This test validates that we count the correct amount of index updates.
+ * <p>
+ * We build the index async with a node scan in the background and consume transactions to keep the index updated. Once
+ * the index is build and becomes online, we save the index size(number of entries) and begin tracking updates. These
+ * values can then then be used to determine when to resample the index for example.
+ *
+ * <pre>
+ *                online
+ *    index size    |      updates
+ *                  v
+ *  |----------------T--------------> stream of transactions
+ *                    ^
+ *    during          |    after
+ *                 observed
+ * </pre>
+ *
+ * Since we observe the index online event without strict synchronization, we cannot determine what transactions are
+ * before and after that event. This is illustrated in the drawing above where transaction {@code T} is counted as
+ * occurring during the population and not after, which is incorrect.
+ * <p>
+ * That is why we allow a difference of {@link IndexStatisticsTest#MISSED_UPDATES_TOLERANCE}, to exists. This threshold
+ * is the number of updates in the larges transaction and that should be the larges error since we check if the index is
+ * online between each transaction.
+ */
 @RunWith( Parameterized.class )
 public class IndexStatisticsTest
 {
@@ -276,6 +301,27 @@ public void shouldProvideIndexStatisticsWhenIndexIsBuiltViaPopulationAndConcurre
         assertCorrectIndexUpdates( updatesTracker.createdAfterPopulation(), indexUpdates( index ) );
     }
 
+    @Test
+    public void shouldProvideIndexStatisticsWhenIndexIsBuiltViaPopulationAndConcurrentAdditionsAndChangesAndDeletions() throws Exception
+    {
+        // given some initial data
+        long[] nodes = repeatCreateNamedPeopleFor( NAMES.length * CREATION_MULTIPLIER );
+        int initialNodes = nodes.length;
+
+        // when populating while creating
+        IndexDescriptor index = createIndex( "Person", "name" );
+        UpdatesTracker updatesTracker = executeCreationsDeletionsAndUpdates( nodes, index, CREATION_MULTIPLIER );
+        awaitIndexesOnline();
+
+        // then
+        int seenWhilePopulating = initialNodes + updatesTracker.createdDuringPopulation() - updatesTracker.deletedDuringPopulation();
+        double expectedSelectivity = UNIQUE_NAMES / seenWhilePopulating;
+        int expectedIndexUpdates = updatesTracker.deletedAfterPopulation() + updatesTracker.createdAfterPopulation() + updatesTracker.updatedAfterPopulation();
+        assertCorrectIndexSelectivity( expectedSelectivity, indexSelectivity( index ) );
+        assertCorrectIndexSize( seenWhilePopulating, indexSize( index ) );
+        assertCorrectIndexUpdates( expectedIndexUpdates, indexUpdates( index ) );
+    }
+
     @Test
     public void shouldWorkWhileHavingHeavyConcurrentUpdates() throws Exception
     {
@@ -330,14 +376,16 @@ private void deleteNode( long nodeId ) throws KernelException
         }
     }
 
-    private boolean changeName( long nodeId, String propertyKeyName, Object newValue ) throws KernelException
+    private boolean changeName( long nodeId, String propertyKeyName, Object newValue )
     {
         boolean changeIndexedNode = false;
         try ( Transaction tx = db.beginTx() )
         {
             Node node = db.getNodeById( nodeId );
-            if ( node.hasProperty( propertyKeyName ) )
+            Object oldValue = node.getProperty( propertyKeyName );
+            if ( !oldValue.equals( newValue ) )
             {
+                // Changes are only propagated when the value actually change
                 changeIndexedNode = true;
             }
             node.setProperty( propertyKeyName, newValue );
@@ -520,7 +568,7 @@ private NeoStores neoStores()
 
     private void awaitIndexesOnline()
     {
-        try ( Transaction transaction = db.beginTx() )
+        try ( Transaction ignored = db.beginTx() )
         {
             db.schema().awaitIndexesOnline(3, TimeUnit.MINUTES );
         }
@@ -590,12 +638,12 @@ private UpdatesTracker internalExecuteCreationsDeletionsAndUpdates( long[] nodes
                 int randomIndex = random.nextInt( nodes.length );
                 try
                 {
-                    if ( changeName( nodes[randomIndex], "name", NAMES[randomIndex % NAMES.length] ) )
+                    if ( changeName( nodes[randomIndex], "name", NAMES[random.nextInt( NAMES.length )] ) )
                     {
                         updatesTracker.increaseUpdated( 1 );
                     }
                 }
-                catch ( EntityNotFoundException | NotFoundException ex )
+                catch ( NotFoundException ex )
                 {
                     // ignore
                 }

community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/UpdatesTracker.java

@@ -128,6 +128,9 @@ public String toString()
                ", createdDuringPopulation=" + createdDuringPopulation +
                ", updatedDuringPopulation=" + updatedDuringPopulation +
                ", deletedDuringPopulation=" + deletedDuringPopulation +
+               ", createdAfterPopulation=" + createdAfterPopulation() +
+               ", updatedAfterPopulation=" + updatedAfterPopulation() +
+               ", deletedAfterPopulation=" + deletedAfterPopulation() +
                ", populationCompleted=" + populationCompleted +
                '}';
     }