Skip to content

Commit

Permalink
More defensive configuration of custom page swappers
Browse files Browse the repository at this point in the history
* We now log when a custom page swapper is being configured.
* If we cannot configure the desired custom page swapper, we now throw an exception to avoid falling back to the default page swapper implementation.
  • Loading branch information
chrisvest committed Jun 27, 2016
1 parent 64d2a8d commit 673ad68
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 12 deletions.
Expand Up @@ -46,13 +46,13 @@ public class ConfiguringPageCacheFactory
public ConfiguringPageCacheFactory( public ConfiguringPageCacheFactory(
FileSystemAbstraction fs, Config config, PageCacheTracer tracer, Log log ) FileSystemAbstraction fs, Config config, PageCacheTracer tracer, Log log )
{ {
this.swapperFactory = createAndConfigureSwapperFactory( fs, config ); this.swapperFactory = createAndConfigureSwapperFactory( fs, config, log );
this.config = config; this.config = config;
this.tracer = tracer; this.tracer = tracer;
this.log = log; this.log = log;
} }


private PageSwapperFactory createAndConfigureSwapperFactory( FileSystemAbstraction fs, Config config ) private PageSwapperFactory createAndConfigureSwapperFactory( FileSystemAbstraction fs, Config config, Log log )
{ {
String desiredImplementation = config.get( pagecache_swapper ); String desiredImplementation = config.get( pagecache_swapper );


Expand All @@ -68,9 +68,11 @@ private PageSwapperFactory createAndConfigureSwapperFactory( FileSystemAbstracti
ConfigurablePageSwapperFactory configurableFactory = (ConfigurablePageSwapperFactory) factory; ConfigurablePageSwapperFactory configurableFactory = (ConfigurablePageSwapperFactory) factory;
configurableFactory.configure( config ); configurableFactory.configure( config );
} }
log.info( "Configured " + pagecache_swapper.name() + ": " + desiredImplementation );
return factory; return factory;
} }
} }
throw new IllegalArgumentException( "Cannot find PageSwapperFactory: " + desiredImplementation );
} }


SingleFilePageSwapperFactory factory = new SingleFilePageSwapperFactory(); SingleFilePageSwapperFactory factory = new SingleFilePageSwapperFactory();
Expand Down
Expand Up @@ -19,32 +19,34 @@
*/ */
package org.neo4j.kernel.impl.pagecache; package org.neo4j.kernel.impl.pagecache;


import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Before; import org.junit.Before;
import org.junit.Rule; import org.junit.Rule;
import org.junit.Test; import org.junit.Test;


import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.io.pagecache.PageCache; import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.impl.SingleFilePageSwapperFactory; import org.neo4j.io.pagecache.impl.SingleFilePageSwapperFactory;
import org.neo4j.io.pagecache.tracing.PageCacheTracer; import org.neo4j.io.pagecache.tracing.PageCacheTracer;
import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Config;
import org.neo4j.logging.BufferingLog;
import org.neo4j.logging.NullLog; import org.neo4j.logging.NullLog;
import org.neo4j.test.EphemeralFileSystemRule; import org.neo4j.test.EphemeralFileSystemRule;


import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;

import static org.neo4j.graphdb.factory.GraphDatabaseSettings.mapped_memory_page_size; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.mapped_memory_page_size;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.pagecache_memory; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.pagecache_memory;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.pagecache_swapper; import static org.neo4j.graphdb.factory.GraphDatabaseSettings.pagecache_swapper;
import static org.neo4j.helpers.collection.MapUtil.stringMap; import static org.neo4j.helpers.collection.MapUtil.stringMap;


public class ConfiguringPageCacheFactoryTest public class ConfiguringPageCacheFactoryTest
{ {
private static final String TEST_IMPL_NAME = "pageswapperForTesting";
@Rule @Rule
public EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule(); public EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule();


Expand Down Expand Up @@ -80,19 +82,33 @@ public void shouldUseConfiguredPageSizeAndFitAsManyPagesAsItCan() throws Throwab
} }


@Test @Test
public void mustUseConfiguredPageSwapper() throws Exception public void mustUseAndLogConfiguredPageSwapper() throws Exception
{ {
// Given // Given
Config config = new Config( stringMap( Config config = new Config( stringMap(
pagecache_memory.name(), "8m", pagecache_memory.name(), "8m",
pagecache_swapper.name(), "test" ) ); pagecache_swapper.name(), TEST_IMPL_NAME ) );
BufferingLog log = new BufferingLog();


// When // When
new ConfiguringPageCacheFactory( fsRule.get(), config, PageCacheTracer.NULL, NullLog.getInstance() ); new ConfiguringPageCacheFactory( fsRule.get(), config, PageCacheTracer.NULL, log );


// Then // Then
assertThat( PageSwapperFactoryForTesting.countCreatedPageSwapperFactories(), is( 1 ) ); assertThat( PageSwapperFactoryForTesting.countCreatedPageSwapperFactories(), is( 1 ) );
assertThat( PageSwapperFactoryForTesting.countConfiguredPageSwapperFactories(), is( 1 ) ); assertThat( PageSwapperFactoryForTesting.countConfiguredPageSwapperFactories(), is( 1 ) );
assertThat( log.toString(), containsString( TEST_IMPL_NAME ) );
}

@Test( expected = IllegalArgumentException.class )
public void mustThrowIfConfiguredPageSwapperCannotBeFound() throws Exception
{
// Given
Config config = new Config( stringMap(
pagecache_memory.name(), "8m",
pagecache_swapper.name(), "non-existing" ) );

// When
new ConfiguringPageCacheFactory( fsRule.get(), config, PageCacheTracer.NULL, NullLog.getInstance() );
} }


@Test @Test
Expand All @@ -102,7 +118,7 @@ public void mustUsePageSwapperCachePageSizeHintAsDefault() throws Exception
int cachePageSizeHint = 16 * 1024; int cachePageSizeHint = 16 * 1024;
PageSwapperFactoryForTesting.cachePageSizeHint.set( cachePageSizeHint ); PageSwapperFactoryForTesting.cachePageSizeHint.set( cachePageSizeHint );
Config config = new Config( stringMap( Config config = new Config( stringMap(
GraphDatabaseSettings.pagecache_swapper.name(), "test" ) ); GraphDatabaseSettings.pagecache_swapper.name(), TEST_IMPL_NAME ) );


// When // When
ConfiguringPageCacheFactory factory = new ConfiguringPageCacheFactory( ConfiguringPageCacheFactory factory = new ConfiguringPageCacheFactory(
Expand All @@ -124,7 +140,7 @@ public void mustIgnoreExplicitlySpecifiedCachePageSizeIfPageSwapperHintIsStrict(
PageSwapperFactoryForTesting.cachePageSizeHintIsStrict.set( true ); PageSwapperFactoryForTesting.cachePageSizeHintIsStrict.set( true );
Config config = new Config( stringMap( Config config = new Config( stringMap(
GraphDatabaseSettings.mapped_memory_page_size.name(), "4096", GraphDatabaseSettings.mapped_memory_page_size.name(), "4096",
GraphDatabaseSettings.pagecache_swapper.name(), "test" ) ); GraphDatabaseSettings.pagecache_swapper.name(), TEST_IMPL_NAME ) );


// When // When
ConfiguringPageCacheFactory factory = new ConfiguringPageCacheFactory( ConfiguringPageCacheFactory factory = new ConfiguringPageCacheFactory(
Expand Down Expand Up @@ -164,7 +180,7 @@ public PageSwapperFactoryForTesting()
@Override @Override
public String implementationName() public String implementationName()
{ {
return "test"; return TEST_IMPL_NAME;
} }


@Override @Override
Expand Down

0 comments on commit 673ad68

Please sign in to comment.