Remove @ignore from label scan store tests,

cleanup todo's in AbstractLuceneIndex, LuceneIndexAccessor and LuceneLabelScanStore
neo4j · Jan 21, 2016 · 1c9629a · 1c9629a
1 parent ad3fdc3
commit 1c9629a
Show file tree

Hide file tree

Showing 33 changed files with 146 additions and 207 deletions.
diff --git a/...cy/src/test/java/org/neo4j/legacy/consistency/ConsistencyCheckServiceIntegrationTest.java b/...cy/src/test/java/org/neo4j/legacy/consistency/ConsistencyCheckServiceIntegrationTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.legacy.consistency;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -53,13 +52,11 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
 import static org.neo4j.legacy.consistency.ConsistencyCheckService.defaultLogFileName;
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be upgraded")
 public class ConsistencyCheckServiceIntegrationTest
 {
     @Test

diff --git a/...c/test/java/org/neo4j/legacy/consistency/checking/full/ExecutionOrderIntegrationTest.java b/...c/test/java/org/neo4j/legacy/consistency/checking/full/ExecutionOrderIntegrationTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.legacy.consistency.checking.full;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.mockito.invocation.InvocationOnMock;
@@ -66,19 +65,16 @@
 import org.neo4j.legacy.consistency.store.RecordAccess;
 import org.neo4j.legacy.consistency.store.RecordReference;
 
+import static java.lang.String.format;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.withSettings;
-
-import static java.lang.String.format;
-
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be upgraded")
 public class ExecutionOrderIntegrationTest
 {
     @Rule

diff --git a/...cy/src/test/java/org/neo4j/legacy/consistency/checking/full/FullCheckIntegrationTest.java b/...cy/src/test/java/org/neo4j/legacy/consistency/checking/full/FullCheckIntegrationTest.java
@@ -133,7 +133,6 @@
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be upgraded")
 @RunWith( Parameterized.class )
 public class FullCheckIntegrationTest
 {

diff --git a/...eck-legacy/src/test/java/org/neo4j/unsafe/impl/batchimport/ParallelBatchImporterTest.java b/...eck-legacy/src/test/java/org/neo4j/unsafe/impl/batchimport/ParallelBatchImporterTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.unsafe.impl.batchimport;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -75,7 +74,6 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-
 import static org.neo4j.helpers.collection.IteratorUtil.asSet;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
 import static org.neo4j.unsafe.impl.batchimport.AdditionalInitialIds.EMPTY;
@@ -86,7 +84,6 @@
 import static org.neo4j.unsafe.impl.batchimport.cache.idmapping.IdMappers.strings;
 import static org.neo4j.unsafe.impl.batchimport.staging.ProcessorAssignmentStrategies.eagerRandomSaturation;
 
-@Ignore("Label scan store to be upgraded")
 @RunWith( Parameterized.class )
 public class ParallelBatchImporterTest
 {

diff --git a/...ncy-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java b/...ncy-check/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceIntegrationTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.consistency;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -52,13 +51,11 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-
 import static org.neo4j.consistency.ConsistencyCheckService.defaultLogFileName;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be upgraded")
 public class ConsistencyCheckServiceIntegrationTest
 {
     @Test

diff --git a/...unity/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java b/...unity/consistency-check/src/test/java/org/neo4j/consistency/ConsistencyCheckToolTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.consistency;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -209,7 +208,6 @@ public void exitWithFailureIfConfigSpecifiedButPropertiesFileDoesNotExist() thro
         verifyZeroInteractions( service );
     }
 
-    @Ignore("Label scan store to be upgraded")
     @Test
     public void shouldExecuteRecoveryWhenStoreWasNonCleanlyShutdown() throws Exception
     {

diff --git a/...heck/src/test/java/org/neo4j/consistency/checking/full/ExecutionOrderIntegrationTest.java b/...heck/src/test/java/org/neo4j/consistency/checking/full/ExecutionOrderIntegrationTest.java
@@ -81,7 +81,6 @@
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be updated")
 public class ExecutionOrderIntegrationTest
 {
     @Rule

diff --git a/...ncy-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java b/...ncy-check/src/test/java/org/neo4j/consistency/checking/full/FullCheckIntegrationTest.java
@@ -139,7 +139,6 @@
 import static org.neo4j.test.Property.property;
 import static org.neo4j.test.Property.set;
 
-@Ignore("Label scan store to be updated")
 public class FullCheckIntegrationTest
 {
     private static final SchemaIndexProvider.Descriptor DESCRIPTOR = new SchemaIndexProvider.Descriptor( "lucene", "1.0" );

diff --git a/...ency-check/src/test/java/org/neo4j/unsafe/impl/batchimport/ParallelBatchImporterTest.java b/...ency-check/src/test/java/org/neo4j/unsafe/impl/batchimport/ParallelBatchImporterTest.java
@@ -19,7 +19,6 @@
  */
 package org.neo4j.unsafe.impl.batchimport;
 
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -74,7 +73,6 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-
 import static org.neo4j.helpers.collection.IteratorUtil.asSet;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
 import static org.neo4j.unsafe.impl.batchimport.AdditionalInitialIds.EMPTY;
@@ -86,13 +84,14 @@
 import static org.neo4j.unsafe.impl.batchimport.input.Collectors.silentBadCollector;
 import static org.neo4j.unsafe.impl.batchimport.staging.ProcessorAssignmentStrategies.eagerRandomSaturation;
 
-@Ignore("Label scan store to be upgraded")
 @RunWith( Parameterized.class )
 public class ParallelBatchImporterTest
 {
+    @Rule
+    public final TargetDirectory.TestDirectory directory = TargetDirectory.testDirForTest( getClass() );
+
     private static final int NODE_COUNT = 10_000;
-    private static final int RELATIONSHIP_COUNT = NODE_COUNT*5;
-    public final @Rule TargetDirectory.TestDirectory directory = TargetDirectory.testDirForTest( getClass() );
+    private static final int RELATIONSHIP_COUNT = NODE_COUNT * 5;
     private final Configuration config = new Configuration.Default()
     {
         @Override

diff --git a/...unity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/AbstractLuceneIndex.java b/...unity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/AbstractLuceneIndex.java
@@ -45,7 +45,6 @@
 
 import static java.util.stream.Collectors.toList;
 
-// todo: this component has an implicit possibility to be opened and closed multiple times. we should revisit and at least test it.
 public abstract class AbstractLuceneIndex implements Closeable
 {
     protected final ReentrantLock commitCloseLock = new ReentrantLock();
@@ -62,6 +61,7 @@ public AbstractLuceneIndex( PartitionedIndexStorage indexStorage )
 
     public void create() throws IOException
     {
+        ensureNotOpen();
         indexStorage.prepareFolder( indexStorage.getIndexFolder() );
         indexStorage.reserveIndexFailureStorage();
         createNewPartitionFolder();
@@ -145,7 +145,12 @@ public void drop() throws IOException
 
     public void flush() throws IOException
     {
-        // TODO: do we need it?
+        List<IndexPartition> partitions = getPartitions();
+        for ( IndexPartition partition : partitions )
+        {
+            partition.getIndexWriter().commit();
+        }
+
     }
 
     @Override
@@ -280,11 +285,25 @@ protected void ensureOpen()
         }
     }
 
+    protected void ensureNotOpen()
+    {
+        if ( open )
+        {
+            throw new IllegalStateException( "Lucene index should not be open to be able to perform required " +
+                                             "operation." );
+        }
+    }
+
     protected boolean hasSinglePartition( List<IndexPartition> partitions )
     {
         return partitions.size() == 1;
     }
 
+    protected IndexPartition getFirstPartition( List<IndexPartition> partitions )
+    {
+        return partitions.get( 0 );
+    }
+
     private boolean luceneDirectoryExists( File folder ) throws IOException
     {
         try ( Directory directory = indexStorage.openDirectory( folder ) )

diff --git a/...unity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessor.java b/...unity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneIndexAccessor.java
@@ -68,8 +68,6 @@ public void drop() throws IOException
     @Override
     public void force() throws IOException
     {
-        // TODO:
-        // do not commit all writers except first
         luceneIndex.markAsOnline();
         luceneIndex.maybeRefreshBlocking();
     }

diff --git a/...nity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneLabelScanIndex.java b/...nity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneLabelScanIndex.java
@@ -58,7 +58,7 @@ public LabelScanReader getLabelScanReader()
             List<IndexPartition> partitions = getPartitions();
             if ( hasSinglePartition( partitions ) )
             {
-                IndexPartition partition = partitions.get( 0 );
+                IndexPartition partition = getFirstPartition( partitions );
                 PartitionSearcher searcher = partition.acquireSearcher();
                 return new SimpleLuceneLabelScanStoreReader( searcher, storageStrategy );
             }
@@ -83,7 +83,7 @@ public LabelScanWriter getLabelScanWriter( Lock heldLock )
             List<IndexPartition> partitions = getPartitions();
             if ( hasSinglePartition( partitions ) )
             {
-                return new SimpleLuceneLabelScanWriter( partitions.get( 0 ), format, heldLock );
+                return new SimpleLuceneLabelScanWriter( getFirstPartition( partitions ), format, heldLock );
             }
             throw new UnsupportedOperationException();
         }

diff --git a/...nity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneLabelScanStore.java b/...nity/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/LuceneLabelScanStore.java
@@ -68,7 +68,7 @@ public void lockedIndex( LockObtainFailedException e )
             }
 
             @Override
-            public void corruptIndex( IOException e )
+            public void corruptedIndex()
             {
             }
 
@@ -89,7 +89,7 @@ public void rebuilt( long roughNodeCount )
 
         void lockedIndex( LockObtainFailedException e );
 
-        void corruptIndex( IOException e );
+        void corruptedIndex();
 
         void rebuilding();
 
@@ -152,19 +152,12 @@ public void init() throws IOException
             }
             else if ( !luceneIndex.isValid() )
             {
-                // todo: rebuild instead of failing? failing is here now only because there is a test expecting failure...
-//                monitor.corruptIndex(  );
-//                luceneIndex.create();
-//                needsRebuild = true;
-
                 log.warn( "Lucene scan store index could not be read. Preparing to rebuild." );
-
-                throw new IOException( "Label scan store could not be read, and needs to be rebuilt. " +
-                                       "To trigger a rebuild, ensure the database is stopped, delete the files in '" +
-                                       luceneIndex + "', and then start the database again." );
+                monitor.corruptedIndex();
+                luceneIndex.drop();
+                luceneIndex.create();
+                needsRebuild = true;
             }
-
-            // todo: test this strange open-close thingy
             luceneIndex.open();
         }
         catch ( LockObtainFailedException e )