Skip to content

Commit

Permalink
Address pr comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
MishaDemianenko committed Nov 21, 2016
1 parent bb2b341 commit 3c5f9d3
Show file tree
Hide file tree
Showing 23 changed files with 103 additions and 104 deletions.
Expand Up @@ -32,7 +32,6 @@

public class RealOutsideWorldTest
{

@Rule
public SystemExitRule systemExitRule = SystemExitRule.none();

Expand Down Expand Up @@ -61,5 +60,4 @@ public void closeFilesystemOnExit() throws IOException

verify( fileSystemMock ).close();
}

}
Expand Up @@ -20,9 +20,11 @@
package org.neo4j.cypher.performance

import java.io.File

import org.neo4j.cypher.internal.frontend.v3_2.test_helpers.CypherFunSuite
import org.neo4j.graphdb.RelationshipType
import org.neo4j.index.impl.lucene.legacy.LuceneBatchInserterIndexProviderNewImpl
import org.neo4j.io.fs.DefaultFileSystemAbstraction
import org.neo4j.unsafe.batchinsert.{BatchInserter, BatchInserterIndex, BatchInserters}

import scala.collection.JavaConverters._
Expand All @@ -41,13 +43,14 @@ class DataImportTest extends CypherFunSuite {

targetDir.exists() should equal(false)

val (inserter, moviesId, moviesTitles, indexProvider, typeIdx) = createInserters(targetDir)
val (fileSystem, inserter, moviesId, moviesTitles, indexProvider, typeIdx) = createInserters(targetDir)

val movieMap = createMovies(sourceDir, inserter, moviesTitles, moviesId, typeIdx)
addRatings(sourceDir, movieMap.toMap, inserter, typeIdx)

indexProvider.shutdown()
inserter.shutdown()
fileSystem.close()
}

private def addRatings(sourceDir: File, movies: Map[String, Long], inserter: BatchInserter, typeIdx: BatchInserterIndex) = {
Expand Down Expand Up @@ -82,14 +85,15 @@ class DataImportTest extends CypherFunSuite {
}

private def createInserters(targetDir: File) = {
val inserter = BatchInserters.inserter(targetDir)
val fileSystem = new DefaultFileSystemAbstraction()
val inserter = BatchInserters.inserter(targetDir, fileSystem)
val indexProvider = new LuceneBatchInserterIndexProviderNewImpl(inserter)

val moviesTitles = createNodeIdx(indexProvider, "movieTitles", "fulltext", "title")
val moviesId = createNodeIdx(indexProvider, "movieIds", "exact", "id")
val typeIdx = createNodeIdx(indexProvider, "type", "exact", "type")

(inserter, moviesId, moviesTitles, indexProvider, typeIdx)
(fileSystem, inserter, moviesId, moviesTitles, indexProvider, typeIdx)
}

private def createMovies(sourceDir: File, inserter: BatchInserter, moviesTitles: BatchInserterIndex, moviesId: BatchInserterIndex, typeIdx: BatchInserterIndex) = {
Expand Down
Expand Up @@ -30,6 +30,7 @@
import org.neo4j.graphalgo.EstimateEvaluator;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.RelationshipType;
import org.neo4j.io.fs.DefaultFileSystemAbstraction;
import org.neo4j.unsafe.batchinsert.BatchInserter;
import org.neo4j.unsafe.batchinsert.BatchInserters;

Expand Down Expand Up @@ -68,24 +69,27 @@ public GeoDataGenerator withMaxNeighborDistance( int maxDistance, double neighbo

public void generate( File storeDir ) throws IOException
{
BatchInserter inserter = BatchInserters.inserter( storeDir.getAbsoluteFile() );
Grid grid = new Grid();
try
try ( DefaultFileSystemAbstraction fileSystem = new DefaultFileSystemAbstraction())
{
for ( int i = 0; i < numberOfNodes; i++ )
BatchInserter inserter = BatchInserters.inserter( storeDir.getAbsoluteFile(), fileSystem );
Grid grid = new Grid();
try
{
grid.createNodeAtRandomLocation( random, inserter );
}
for ( int i = 0; i < numberOfNodes; i++ )
{
grid.createNodeAtRandomLocation( random, inserter );
}

for ( int i = 0; i < numberOfConnections; i++ )
for ( int i = 0; i < numberOfConnections; i++ )
{
grid.createConnection( random, inserter );
}
}
finally
{
grid.createConnection( random, inserter );
inserter.shutdown();
}
}
finally
{
inserter.shutdown();
}
}

private static class PositionedNode
Expand Down
Expand Up @@ -450,7 +450,8 @@ public static void doImport( PrintStream out, PrintStream err, File storeDir, Fi
LogService logService = life.add( StoreLogService.inLogsDirectory( fs, logsDir ) );

life.start();
BatchImporter importer = new ParallelBatchImporter( storeDir, fs,
BatchImporter importer = new ParallelBatchImporter( storeDir,
fs,
configuration,
logService,
ExecutionMonitors.defaultVisible(),
Expand Down
Expand Up @@ -24,8 +24,8 @@
import org.neo4j.io.fs.FileSystemAbstraction;

/**
* File system abstraction that wraps real file system and prevent it to be closed.
* Useful for cases when we pass file system to db, shutdown it and verify file system content.
* File system abstraction that wraps real file system and prevent it from being closed.
* Useful for cases when we pass file system to db, shut it down and verify file system content.
*/
public class UncloseableDelegatingFileSystemAbstraction extends DelegatingFileSystemAbstraction
{
Expand Down
Expand Up @@ -103,10 +103,20 @@ protected File getFile()
}

protected FileSystemAbstraction getFs()
{
return getEphemeralFileSystem();
}

protected FileSystemAbstraction getEphemeralFileSystem()
{
return ephemeralFileSystem;
}

protected FileSystemAbstraction getRealFileSystem()
{
return fileSystem;
}

protected void assumeFalse( String message, boolean test )
{
if ( test )
Expand Down
Expand Up @@ -20,29 +20,11 @@
package org.neo4j.io.pagecache.impl;

import java.io.File;
import java.io.IOException;

import org.neo4j.io.fs.DefaultFileSystemAbstraction;
import org.neo4j.io.fs.FileSystemAbstraction;

public class SingleFilePageSwapperWithRealFileSystemIT extends SingleFilePageSwapperTest
{
private DefaultFileSystemAbstraction fs;

@Override
public void setUp() throws IOException
{
super.setUp();
fs = new DefaultFileSystemAbstraction();
}

@Override
public void tearDown() throws IOException
{
// Do nothing
fs.close();
}

@Override
protected File getFile()
{
Expand All @@ -52,6 +34,6 @@ protected File getFile()
@Override
protected FileSystemAbstraction getFs()
{
return fs;
return getRealFileSystem();
}
}
Expand Up @@ -24,7 +24,6 @@
import java.util.Map;

import org.neo4j.helpers.Service;
import org.neo4j.io.fs.DefaultFileSystemAbstraction;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.extension.KernelExtensionFactory;
import org.neo4j.unsafe.batchinsert.internal.BatchInserterImpl;
Expand All @@ -36,50 +35,24 @@
*/
public final class BatchInserters
{
/**
* Get a {@link BatchInserter} given a store directory.
*
* @param storeDir the store directory
* @return a new {@link BatchInserter}
* @throws IOException if there is an IO error
*/
public static BatchInserter inserter( File storeDir ) throws IOException
{
return inserter( storeDir, stringMap() );
}

public static BatchInserter inserter( File storeDir, FileSystemAbstraction fs ) throws IOException
{
return inserter( storeDir, fs, stringMap(), (Iterable) Service.load( KernelExtensionFactory.class ) );
}

/**
* Get a {@link BatchInserter} given a store directory.
*
* @param storeDir the store directory
* @param config configuration settings to use
* @return a new {@link BatchInserter}
* @throws IOException if there is an IO error
*/
public static BatchInserter inserter( File storeDir, Map<String,String> config ) throws IOException
{
return inserter( storeDir, new DefaultFileSystemAbstraction(), config, (Iterable) Service.load( KernelExtensionFactory.class ) );
return inserter( storeDir, fs, stringMap(), loadKernelExtension() );
}

public static BatchInserter inserter( File storeDir, FileSystemAbstraction fs, Map<String,String> config ) throws IOException
{
return inserter( storeDir, fs, config, (Iterable) Service.load( KernelExtensionFactory.class ) );
return inserter( storeDir, fs, config, loadKernelExtension() );
}

public static BatchInserter inserter( File storeDir,
public static BatchInserter inserter( File storeDir, FileSystemAbstraction fileSystem,
Map<String, String> config, Iterable<KernelExtensionFactory<?>> kernelExtensions ) throws IOException
{
return new BatchInserterImpl( storeDir, new DefaultFileSystemAbstraction(), config, kernelExtensions );
return new BatchInserterImpl( storeDir, fileSystem, config, kernelExtensions );
}

public static BatchInserter inserter( File storeDir, FileSystemAbstraction fileSystem,
Map<String, String> config, Iterable<KernelExtensionFactory<?>> kernelExtensions ) throws IOException
private static Iterable loadKernelExtension()
{
return new BatchInserterImpl( storeDir, fileSystem, config, kernelExtensions );
return Service.load( KernelExtensionFactory.class );
}
}
Expand Up @@ -49,7 +49,6 @@
import org.neo4j.helpers.collection.Iterators;
import org.neo4j.helpers.collection.Visitor;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.io.fs.FileSystemLifecycleAdapter;
import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.tracing.PageCacheTracer;
import org.neo4j.kernel.api.constraints.NodePropertyExistenceConstraint;
Expand Down Expand Up @@ -224,7 +223,6 @@ public BatchInserterImpl( final File storeDir, final FileSystemAbstraction fileS

life = new LifeSupport();
this.storeDir = storeDir;
life.add( new FileSystemLifecycleAdapter( fileSystem ) );
ConfiguringPageCacheFactory pageCacheFactory = new ConfiguringPageCacheFactory(
fileSystem, config, PageCacheTracer.NULL, NullLog.getInstance() );
PageCache pageCache = pageCacheFactory.getOrCreatePageCache();
Expand Down
Expand Up @@ -29,7 +29,6 @@
import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.Node;
import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.mockfs.UncloseableDelegatingFileSystemAbstraction;
import org.neo4j.helpers.collection.Iterables;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.test.TestGraphDatabaseFactory;
Expand All @@ -50,7 +49,7 @@ public void lazyLoadWithinWriteTransaction() throws Exception
// Given
FileSystemAbstraction fileSystem = fs.get();
File dir = new File( "dir" ).getAbsoluteFile();
BatchInserter inserter = BatchInserters.inserter( dir, new UncloseableDelegatingFileSystemAbstraction( fileSystem ) );
BatchInserter inserter = BatchInserters.inserter( dir, fileSystem );
int count = 3000;
long nodeId = inserter.createNode( mapWithManyProperties( count /* larger than initial property index load threshold */ ) );
inserter.shutdown();
Expand Down
Expand Up @@ -25,18 +25,21 @@

import java.util.Collections;

import org.neo4j.test.rule.fs.DefaultFileSystemRule;
import org.neo4j.unsafe.batchinsert.BatchInserter;
import org.neo4j.unsafe.batchinsert.BatchInserters;

public class GitHub1304Test
{
@Rule
public TemporaryFolder folder = new TemporaryFolder();
@Rule
public DefaultFileSystemRule fileSystemRule = new DefaultFileSystemRule();

@Test
public void givenBatchInserterWhenArrayPropertyUpdated4TimesThenShouldNotFail() throws Exception
{
BatchInserter batchInserter = BatchInserters.inserter( folder.getRoot().getAbsoluteFile() );
BatchInserter batchInserter = BatchInserters.inserter( folder.getRoot().getAbsoluteFile(), fileSystemRule.get() );

long nodeId = batchInserter.createNode( Collections.<String, Object>emptyMap() );

Expand Down
Expand Up @@ -28,6 +28,7 @@
import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.graphdb.mockfs.UncloseableDelegatingFileSystemAbstraction;
import org.neo4j.graphdb.security.URLAccessRule;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.kernel.GraphDatabaseDependencies;
Expand All @@ -47,7 +48,11 @@
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.boltConnector;

/**
* Test factory for graph databases
* Test factory for graph databases.
* Please be aware that since it's a database it will close filesystem as part of it lifecycle
* a real database.
* If you expect your file system to be open in the end use {@link UncloseableDelegatingFileSystemAbstraction}
*
*/
public class TestGraphDatabaseFactory extends GraphDatabaseFactory
{
Expand Down
Expand Up @@ -632,7 +632,7 @@ public void createBatchNodeAndRelationshipsDeleteAllInEmbedded() throws Exceptio
public void messagesLogGetsClosed() throws Exception
{
File storeDir = this.storeDir.graphDbDir();
BatchInserter inserter = BatchInserters.inserter( storeDir, stringMap() );
BatchInserter inserter = BatchInserters.inserter( storeDir, fileSystemRule.get(), stringMap() );
inserter.shutdown();
assertTrue( new File( storeDir, StoreLogService.INTERNAL_LOG_NAME ).delete() );
}
Expand Down
Expand Up @@ -22,6 +22,7 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.RuleChain;

import java.io.File;
import java.io.IOException;
Expand All @@ -35,6 +36,7 @@
import org.neo4j.kernel.internal.StoreLocker;
import org.neo4j.test.ReflectionUtil;
import org.neo4j.test.rule.TestDirectory;
import org.neo4j.test.rule.fs.DefaultFileSystemRule;
import org.neo4j.unsafe.batchinsert.BatchInserter;
import org.neo4j.unsafe.batchinsert.BatchInserters;

Expand All @@ -45,16 +47,19 @@

public class BatchInserterImplTest
{
private final TestDirectory testDirectory = TestDirectory.testDirectory();
private final ExpectedException expected = ExpectedException.none();
private final DefaultFileSystemRule fileSystemRule = new DefaultFileSystemRule();

@Rule
public TestDirectory testDirectory = TestDirectory.testDirectory();
@Rule
public ExpectedException expected = ExpectedException.none();
public final RuleChain ruleChain = RuleChain.outerRule( testDirectory )
.around( expected ).around( fileSystemRule );

@Test
public void testHonorsPassedInParams() throws Exception
{
BatchInserter inserter = BatchInserters.inserter( testDirectory.graphDbDir(), stringMap(
GraphDatabaseSettings.pagecache_memory.name(), "280K",
BatchInserter inserter = BatchInserters.inserter( testDirectory.graphDbDir(), fileSystemRule.get(),
stringMap( GraphDatabaseSettings.pagecache_memory.name(), "280K",
GraphDatabaseSettings.mapped_memory_page_size.name(), "1K" ) );
NeoStores neoStores = ReflectionUtil.getPrivateField( inserter, "neoStores", NeoStores.class );
PageCache pageCache = ReflectionUtil.getPrivateField( neoStores, "pageCache", PageCache.class );
Expand All @@ -70,7 +75,7 @@ public void testCreatesStoreLockFile() throws Exception
File file = testDirectory.graphDbDir();

// When
BatchInserter inserter = BatchInserters.inserter( file.getAbsoluteFile() );
BatchInserter inserter = BatchInserters.inserter( file.getAbsoluteFile(), fileSystemRule.get() );

// Then
assertThat( new File( file, StoreLocker.STORE_LOCK_FILENAME ).exists(), equalTo( true ) );
Expand All @@ -91,7 +96,7 @@ public void testFailsOnExistingStoreLockFile() throws IOException
expected.expect( StoreLockException.class );
expected.expectMessage( "Unable to obtain lock on store lock file" );
// When
BatchInserters.inserter( parent.getAbsoluteFile() );
BatchInserters.inserter( parent.getAbsoluteFile(), fileSystemAbstraction );
}
}
}

0 comments on commit 3c5f9d3

Please sign in to comment.