GenericNativeIndex use IndexDropAction for Accessor and Populator

IndexDropAction moves into three classes:
- BlockBasedIndexPopulator
- GenericNativeIndexAccessor
- GenericNativeIndexPopulator

Behaviour change:
BlockBasedIndexPopulator and GenericNativeIndexPopulator now delete whole
index directory on drop().

This was already the case for GenericNativeIndexAccessor.

community/community-it/index-it/src/test/java/org/neo4j/kernel/impl/api/constraints/ConstraintCreationIT.java

@@ -74,7 +74,7 @@ public void shouldNotLeaveNativeIndexFilesHangingAroundIfConstraintCreationFails
                 db.getDependencyResolver().resolveDependency( IndexProviderMap.class ).getDefaultProvider();
         File indexDir = indexProvider.directoryStructure().directoryForIndex( indexId );
 
-        assertEquals( 0, indexDir.listFiles().length );
+        assertFalse( indexDir.exists() );
     }
 
     private void attemptAndFailConstraintCreation()

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/BlockBasedIndexPopulator.java

@@ -59,6 +59,7 @@
 import org.neo4j.values.storable.Value;
 
 import static org.neo4j.helpers.collection.Iterables.first;
+import static org.neo4j.kernel.impl.index.schema.BlockStorage.Monitor.NO_MONITOR;
 import static org.neo4j.kernel.impl.index.schema.NativeIndexUpdater.initializeKeyFromUpdate;
 import static org.neo4j.kernel.impl.index.schema.NativeIndexes.deleteIndex;
 
@@ -86,6 +87,7 @@ public abstract class BlockBasedIndexPopulator<KEY extends NativeIndexKey<KEY>,V
     private static final int MERGE_FACTOR = FeatureToggles.getInteger( BlockBasedIndexPopulator.class, "mergeFactor", 8 );
 
     private final IndexDirectoryStructure directoryStructure;
+    private final IndexDropAction dropAction;
     private final boolean archiveFailedIndex;
     private final int blockSize;
     private final int mergeFactor;
@@ -107,18 +109,20 @@ public abstract class BlockBasedIndexPopulator<KEY extends NativeIndexKey<KEY>,V
 
     BlockBasedIndexPopulator( PageCache pageCache, FileSystemAbstraction fs, File file, IndexLayout<KEY,VALUE> layout, IndexProvider.Monitor monitor,
             StoreIndexDescriptor descriptor, IndexSpecificSpaceFillingCurveSettingsCache spatialSettings,
-            IndexDirectoryStructure directoryStructure, boolean archiveFailedIndex )
+            IndexDirectoryStructure directoryStructure, IndexDropAction dropAction, boolean archiveFailedIndex )
     {
-        this( pageCache, fs, file, layout, monitor, descriptor, spatialSettings, directoryStructure, archiveFailedIndex, parseBlockSize(), MERGE_FACTOR,
-                BlockStorage.Monitor.NO_MONITOR );
+        this( pageCache, fs, file, layout, monitor, descriptor, spatialSettings, directoryStructure, dropAction, archiveFailedIndex, parseBlockSize(),
+                MERGE_FACTOR, NO_MONITOR );
     }
 
     BlockBasedIndexPopulator( PageCache pageCache, FileSystemAbstraction fs, File file, IndexLayout<KEY,VALUE> layout, IndexProvider.Monitor monitor,
             StoreIndexDescriptor descriptor, IndexSpecificSpaceFillingCurveSettingsCache spatialSettings,
-            IndexDirectoryStructure directoryStructure, boolean archiveFailedIndex, int blockSize, int mergeFactor, BlockStorage.Monitor blockStorageMonitor )
+            IndexDirectoryStructure directoryStructure, IndexDropAction dropAction, boolean archiveFailedIndex,
+            int blockSize, int mergeFactor, BlockStorage.Monitor blockStorageMonitor )
     {
         super( pageCache, fs, file, layout, monitor, descriptor, new SpaceFillingCurveSettingsWriter( spatialSettings ) );
         this.directoryStructure = directoryStructure;
+        this.dropAction = dropAction;
         this.archiveFailedIndex = archiveFailedIndex;
         this.blockSize = blockSize;
         this.mergeFactor = mergeFactor;
@@ -471,11 +475,15 @@ public synchronized void drop()
     {
         try
         {
+            // Close internal resources
             closeBlockStorage();
         }
         finally
         {
+            // Super drop will close inherited resources
             super.drop();
+            // Cleanup files
+            dropAction.drop( descriptor.getId(), archiveFailedIndex );
         }
     }
 

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericBlockBasedIndexPopulator.java

@@ -29,16 +29,17 @@
 import org.neo4j.kernel.impl.index.schema.config.IndexSpecificSpaceFillingCurveSettingsCache;
 import org.neo4j.storageengine.api.schema.StoreIndexDescriptor;
 
-public class GenericBlockBasedIndexPopulator extends BlockBasedIndexPopulator<GenericKey,NativeIndexValue>
+class GenericBlockBasedIndexPopulator extends BlockBasedIndexPopulator<GenericKey,NativeIndexValue>
 {
     private final IndexSpecificSpaceFillingCurveSettingsCache spatialSettings;
     private final SpaceFillingCurveConfiguration configuration;
 
     GenericBlockBasedIndexPopulator( PageCache pageCache, FileSystemAbstraction fs, File file, IndexLayout<GenericKey,NativeIndexValue> layout,
             IndexProvider.Monitor monitor, StoreIndexDescriptor descriptor, IndexSpecificSpaceFillingCurveSettingsCache spatialSettings,
-            IndexDirectoryStructure directoryStructure, SpaceFillingCurveConfiguration configuration, boolean archiveFailedIndex )
+            IndexDirectoryStructure directoryStructure, SpaceFillingCurveConfiguration configuration,
+            IndexDropAction dropAction, boolean archiveFailedIndex )
     {
-        super( pageCache, fs, file, layout, monitor, descriptor, spatialSettings, directoryStructure, archiveFailedIndex );
+        super( pageCache, fs, file, layout, monitor, descriptor, spatialSettings, directoryStructure, dropAction, archiveFailedIndex );
         this.spatialSettings = spatialSettings;
         this.configuration = configuration;
     }

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexAccessor.java

@@ -20,16 +20,13 @@
 package org.neo4j.kernel.impl.index.schema;
 
 import java.io.File;
-import java.io.IOException;
-import java.io.UncheckedIOException;
 
 import org.neo4j.gis.spatial.index.curves.SpaceFillingCurveConfiguration;
 import org.neo4j.index.internal.gbptree.GBPTree;
 import org.neo4j.index.internal.gbptree.RecoveryCleanupWorkCollector;
 import org.neo4j.io.fs.FileSystemAbstraction;
 import org.neo4j.io.pagecache.IOLimiter;
 import org.neo4j.io.pagecache.PageCache;
-import org.neo4j.kernel.api.index.IndexDirectoryStructure;
 import org.neo4j.kernel.api.index.IndexProvider;
 import org.neo4j.kernel.impl.index.schema.config.IndexSpecificSpaceFillingCurveSettingsCache;
 import org.neo4j.kernel.impl.index.schema.config.SpaceFillingCurveSettingsWriter;
@@ -41,34 +38,26 @@
 class GenericNativeIndexAccessor extends NativeIndexAccessor<GenericKey,NativeIndexValue>
 {
     private final IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings;
-    private final IndexDirectoryStructure directoryStructure;
     private final SpaceFillingCurveConfiguration configuration;
+    private final IndexDropAction dropAction;
     private Validator<Value[]> validator;
 
     GenericNativeIndexAccessor( PageCache pageCache, FileSystemAbstraction fs, File storeFile, IndexLayout<GenericKey,NativeIndexValue> layout,
             RecoveryCleanupWorkCollector recoveryCleanupWorkCollector, IndexProvider.Monitor monitor, StoreIndexDescriptor descriptor,
-            IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings, IndexDirectoryStructure directoryStructure,
-            SpaceFillingCurveConfiguration configuration )
+            IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings, SpaceFillingCurveConfiguration configuration, IndexDropAction dropAction )
     {
         super( pageCache, fs, storeFile, layout, monitor, descriptor, new SpaceFillingCurveSettingsWriter( spaceFillingCurveSettings ) );
         this.spaceFillingCurveSettings = spaceFillingCurveSettings;
-        this.directoryStructure = directoryStructure;
         this.configuration = configuration;
+        this.dropAction = dropAction;
         instantiateTree( recoveryCleanupWorkCollector, headerWriter );
     }
 
     @Override
     public void drop()
     {
         super.drop();
-        try
-        {
-            NativeIndexes.deleteIndex( fileSystem, directoryStructure, descriptor.getId(), false );
-        }
-        catch ( IOException e )
-        {
-            throw new UncheckedIOException( e );
-        }
+        dropAction.drop( descriptor.getId(), false );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexPopulator.java

@@ -39,16 +39,18 @@ class GenericNativeIndexPopulator extends NativeIndexPopulator<GenericKey,Native
     private final IndexSpecificSpaceFillingCurveSettingsCache spatialSettings;
     private final IndexDirectoryStructure directoryStructure;
     private final SpaceFillingCurveConfiguration configuration;
+    private final IndexDropAction dropAction;
     private final boolean archiveFailedIndex;
 
     GenericNativeIndexPopulator( PageCache pageCache, FileSystemAbstraction fs, File storeFile, IndexLayout<GenericKey,NativeIndexValue> layout,
             IndexProvider.Monitor monitor, StoreIndexDescriptor descriptor, IndexSpecificSpaceFillingCurveSettingsCache spatialSettings,
-            IndexDirectoryStructure directoryStructure, SpaceFillingCurveConfiguration configuration, boolean archiveFailedIndex )
+            IndexDirectoryStructure directoryStructure, SpaceFillingCurveConfiguration configuration, IndexDropAction dropAction, boolean archiveFailedIndex )
     {
         super( pageCache, fs, storeFile, layout, monitor, descriptor, new SpaceFillingCurveSettingsWriter( spatialSettings ) );
         this.spatialSettings = spatialSettings;
         this.directoryStructure = directoryStructure;
         this.configuration = configuration;
+        this.dropAction = dropAction;
         this.archiveFailedIndex = archiveFailedIndex;
     }
 
@@ -70,6 +72,15 @@ public void create()
         }
     }
 
+    @Override
+    public synchronized void drop()
+    {
+        // Close resources
+        super.drop();
+        // Cleanup directory
+        dropAction.drop( descriptor.getId(), archiveFailedIndex );
+    }
+
     @Override
     NativeIndexReader<GenericKey,NativeIndexValue> newReader()
     {

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexProvider.java

@@ -122,6 +122,7 @@ public class GenericNativeIndexProvider extends NativeIndexProvider<GenericKey,N
      */
     private final SpaceFillingCurveConfiguration configuration;
     private final boolean archiveFailedIndex;
+    private final IndexDropAction dropAction;
 
     GenericNativeIndexProvider( IndexDirectoryStructure.Factory directoryStructureFactory, PageCache pageCache, FileSystemAbstraction fs, Monitor monitor,
             RecoveryCleanupWorkCollector recoveryCleanupWorkCollector, boolean readOnly, Config config )
@@ -131,6 +132,7 @@ public class GenericNativeIndexProvider extends NativeIndexProvider<GenericKey,N
         this.configuredSettings = new ConfiguredSpaceFillingCurveSettingsCache( config );
         this.configuration = getConfiguredSpaceFillingCurveConfiguration( config );
         this.archiveFailedIndex = config.get( GraphDatabaseSettings.archive_failed_index );
+        this.dropAction = new FileSystemIndexDropAction( fs, directoryStructure() );
     }
 
     @Override
@@ -159,18 +161,18 @@ protected IndexPopulator newIndexPopulator( File storeFile, GenericLayout layout
         if ( blockBasedPopulation )
         {
             return new GenericBlockBasedIndexPopulator( pageCache, fs, storeFile, layout, monitor, descriptor, layout.getSpaceFillingCurveSettings(),
-                    directoryStructure(), configuration, archiveFailedIndex );
+                    directoryStructure(), configuration, dropAction, archiveFailedIndex );
         }
         return new WorkSyncedNativeIndexPopulator<>(
                 new GenericNativeIndexPopulator( pageCache, fs, storeFile, layout, monitor, descriptor, layout.getSpaceFillingCurveSettings(),
-                        directoryStructure(), configuration, archiveFailedIndex ) );
+                        directoryStructure(), configuration, dropAction, archiveFailedIndex ) );
     }
 
     @Override
     protected IndexAccessor newIndexAccessor( File storeFile, GenericLayout layout, StoreIndexDescriptor descriptor )
     {
         return new GenericNativeIndexAccessor( pageCache, fs, storeFile, layout, recoveryCleanupWorkCollector, monitor, descriptor,
-                layout.getSpaceFillingCurveSettings(), directoryStructure(), configuration );
+                layout.getSpaceFillingCurveSettings(), configuration, dropAction );
     }
 
     @Override

community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/BlockBasedIndexPopulatorTest.java

@@ -19,9 +19,11 @@
  */
 package org.neo4j.kernel.impl.index.schema;
 
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.io.File;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
@@ -30,12 +32,11 @@
 
 import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
 import org.neo4j.internal.kernel.api.schema.IndexProviderDescriptor;
+import org.neo4j.io.fs.FileSystemAbstraction;
 import org.neo4j.kernel.api.index.IndexDirectoryStructure;
 import org.neo4j.kernel.api.index.IndexEntryUpdate;
-import org.neo4j.kernel.api.index.IndexProvider;
 import org.neo4j.kernel.api.schema.SchemaDescriptorFactory;
 import org.neo4j.kernel.configuration.Config;
-import org.neo4j.kernel.impl.api.index.PhaseTracker;
 import org.neo4j.kernel.impl.index.schema.config.ConfiguredSpaceFillingCurveSettingsCache;
 import org.neo4j.kernel.impl.index.schema.config.IndexSpecificSpaceFillingCurveSettingsCache;
 import org.neo4j.storageengine.api.schema.IndexDescriptorFactory;
@@ -47,8 +48,12 @@
 import org.neo4j.test.rule.concurrent.OtherThreadRule;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.neo4j.kernel.api.index.IndexDirectoryStructure.directoriesByProvider;
+import static org.neo4j.kernel.api.index.IndexProvider.Monitor.EMPTY;
+import static org.neo4j.kernel.impl.api.index.PhaseTracker.nullInstance;
+import static org.neo4j.kernel.impl.index.schema.BlockStorage.Monitor.NO_MONITOR;
 import static org.neo4j.test.OtherThreadExecutor.command;
 import static org.neo4j.test.Race.throwing;
 import static org.neo4j.values.storable.Values.stringValue;
@@ -66,6 +71,23 @@ public class BlockBasedIndexPopulatorTest
     @Rule
     public final OtherThreadRule<Void> t3 = new OtherThreadRule<>( "CLOSER" );
 
+    private IndexDirectoryStructure directoryStructure;
+    private File indexDir;
+    private File indexFile;
+    private FileSystemAbstraction fs;
+    private IndexDropAction dropAction;
+
+    @Before
+    public void setup()
+    {
+        IndexProviderDescriptor providerDescriptor = new IndexProviderDescriptor( "test", "v1" );
+        directoryStructure = directoriesByProvider( storage.directory().databaseDir() ).forProvider( providerDescriptor );
+        indexDir = directoryStructure.directoryForIndex( INDEX_DESCRIPTOR.getId() );
+        indexFile = new File( indexDir, "index" );
+        fs = storage.fileSystem();
+        dropAction = new FileSystemIndexDropAction( fs, directoryStructure );
+    }
+
     @Test
     public void shouldAwaitMergeToBeFullyAbortedBeforeLeavingCloseMethod() throws Exception
     {
@@ -74,7 +96,7 @@ public void shouldAwaitMergeToBeFullyAbortedBeforeLeavingCloseMethod() throws Ex
         BlockBasedIndexPopulator<GenericKey,NativeIndexValue> populator = instantiatePopulatorWithSomeData( monitor );
 
         // when starting to merge (in a separate thread)
-        Future<Object> mergeFuture = t2.execute( command( () -> populator.scanCompleted( PhaseTracker.nullInstance ) ) );
+        Future<Object> mergeFuture = t2.execute( command( () -> populator.scanCompleted( nullInstance ) ) );
         // and waiting for merge to get going
         monitor.barrier.awaitUninterruptibly();
         // calling close here should wait for the merge future, so that checking the merge future for "done" immediately afterwards must say true
@@ -96,7 +118,7 @@ public void shouldReportAccurateProgressThroughoutThePhases() throws Exception
         try
         {
             // when starting to merge (in a separate thread)
-            Future<Object> mergeFuture = t2.execute( command( () -> populator.scanCompleted( PhaseTracker.nullInstance ) ) );
+            Future<Object> mergeFuture = t2.execute( command( () -> populator.scanCompleted( nullInstance ) ) );
             // and waiting for merge to get going
             monitor.barrier.awaitUninterruptibly();
             // this is a bit fuzzy, but what we want is to assert that the scan doesn't represent 100% of the work
@@ -118,30 +140,54 @@ public void shouldReportAccurateProgressThroughoutThePhases() throws Exception
     public void shouldCorrectlyDecideToAwaitMergeDependingOnProgress() throws Throwable
     {
         // given
-        BlockBasedIndexPopulator<GenericKey,NativeIndexValue> populator = instantiatePopulatorWithSomeData( BlockStorage.Monitor.NO_MONITOR );
+        BlockBasedIndexPopulator<GenericKey,NativeIndexValue> populator = instantiatePopulatorWithSomeData( NO_MONITOR );
 
         // when
         Race race = new Race();
-        race.addContestant( throwing( () -> populator.scanCompleted( PhaseTracker.nullInstance ) ) );
+        race.addContestant( throwing( () -> populator.scanCompleted( nullInstance ) ) );
         race.addContestant( throwing( () -> populator.close( false ) ) );
         race.go();
 
         // then regardless of who wins (close/merge) after close call returns no files should still be mapped
-        EphemeralFileSystemAbstraction ephemeralFileSystem = (EphemeralFileSystemAbstraction) storage.fileSystem();
+        EphemeralFileSystemAbstraction ephemeralFileSystem = (EphemeralFileSystemAbstraction) fs;
         ephemeralFileSystem.assertNoOpenFiles();
     }
 
+    @Test
+    public void shouldDeleteDirectoryOnDrop() throws Exception
+    {
+        // given
+        TrappingMonitor monitor = new TrappingMonitor( false );
+        BlockBasedIndexPopulator<GenericKey,NativeIndexValue> populator = instantiatePopulatorWithSomeData( monitor );
+
+        // when starting to merge (in a separate thread)
+        Future<Object> mergeFuture = t2.execute( command( () -> populator.scanCompleted( nullInstance ) ) );
+        // and waiting for merge to get going
+        monitor.barrier.awaitUninterruptibly();
+        // calling drop here should wait for the merge future and then delete index directory
+        assertTrue( fs.fileExists( indexDir ) );
+        assertTrue( fs.isDirectory( indexDir ) );
+        assertTrue( fs.listFiles( indexDir ).length > 0 );
+
+        Future<Object> dropFuture = t3.execute( command( populator::drop ) );
+        t3.get().waitUntilWaiting();
+        monitor.barrier.release();
+        dropFuture.get();
+
+        // then
+        assertTrue( mergeFuture.isDone() );
+        assertFalse( fs.fileExists( indexDir ) );
+    }
+
     private BlockBasedIndexPopulator<GenericKey,NativeIndexValue> instantiatePopulatorWithSomeData( BlockStorage.Monitor monitor )
     {
-        IndexProviderDescriptor providerDescriptor = new IndexProviderDescriptor( "test", "v1" );
         Config config = Config.defaults();
-        IndexSpecificSpaceFillingCurveSettingsCache spatialSettings =
-                new IndexSpecificSpaceFillingCurveSettingsCache( new ConfiguredSpaceFillingCurveSettingsCache( config ), new HashMap<>() );
+        ConfiguredSpaceFillingCurveSettingsCache settingsCache = new ConfiguredSpaceFillingCurveSettingsCache( config );
+        IndexSpecificSpaceFillingCurveSettingsCache spatialSettings = new IndexSpecificSpaceFillingCurveSettingsCache( settingsCache, new HashMap<>() );
         GenericLayout layout = new GenericLayout( 1, spatialSettings );
-        IndexDirectoryStructure directoryStructure = directoriesByProvider( storage.directory().directory( "schema" ) ).forProvider( providerDescriptor );
         BlockBasedIndexPopulator<GenericKey,NativeIndexValue> populator =
-                new BlockBasedIndexPopulator<GenericKey,NativeIndexValue>( storage.pageCache(), storage.fileSystem(), storage.directory().file( "file" ),
-                        layout, IndexProvider.Monitor.EMPTY, INDEX_DESCRIPTOR, spatialSettings, directoryStructure, false, 100, 2, monitor )
+                new BlockBasedIndexPopulator<GenericKey,NativeIndexValue>( storage.pageCache(), fs, indexFile, layout, EMPTY,
+                        INDEX_DESCRIPTOR, spatialSettings, directoryStructure, dropAction, false, 100, 2, monitor )
                 {
                     @Override
                     NativeIndexReader<GenericKey,NativeIndexValue> newReader()