Skip to content

Commit

Permalink
Fix flakiness of PageCacheTest.backgroundThreadsMustGracefullyShutDown
Browse files Browse the repository at this point in the history
The test was relying on `System.gc()` to observe that we don't keep live references to internal data structures in the page cache after shut down.
This was quite unreliable, so now we additionally produce a ton of old-gen heap pollution, as a mechanism to provoke "natural" old-gen garbage collections.
  • Loading branch information
chrisvest committed Jun 9, 2016
1 parent 6a72076 commit 044eeab
Showing 1 changed file with 30 additions and 24 deletions.
Expand Up @@ -19,7 +19,6 @@
*/
package org.neo4j.io.pagecache;

import org.apache.commons.lang3.SystemUtils;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
Expand Down Expand Up @@ -65,6 +64,7 @@
import org.neo4j.graphdb.mockfs.DelegatingFileSystemAbstraction;
import org.neo4j.graphdb.mockfs.DelegatingStoreChannel;
import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
import org.neo4j.io.ByteUnit;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.io.fs.StoreChannel;
import org.neo4j.io.pagecache.impl.SingleFilePageSwapperFactory;
Expand All @@ -81,6 +81,8 @@
import org.neo4j.test.LinearHistoryPageCacheTracer;
import org.neo4j.test.RepeatRule;

import static java.lang.Long.toHexString;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static org.hamcrest.Matchers.both;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
Expand All @@ -93,10 +95,6 @@
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import static java.lang.Long.toHexString;
import static java.util.concurrent.TimeUnit.MILLISECONDS;

import static org.neo4j.io.pagecache.PagedFile.PF_EXCLUSIVE_LOCK;
import static org.neo4j.io.pagecache.PagedFile.PF_NO_FAULT;
import static org.neo4j.io.pagecache.PagedFile.PF_NO_GROW;
Expand Down Expand Up @@ -3552,8 +3550,6 @@ public void run(
@Test( timeout = SEMI_LONG_TIMEOUT_MILLIS )
public void backgroundThreadsMustGracefullyShutDown() throws Exception
{
assumeTrue( "For some reason, this test is very flaky on Windows", !SystemUtils.IS_OS_WINDOWS );

int iterations = 1000;
List<WeakReference<PageCache>> refs = new LinkedList<>();
final Queue<Throwable> caughtExceptions = new ConcurrentLinkedQueue<>();
Expand All @@ -3576,22 +3572,7 @@ public void uncaughtException( Thread t, Throwable e )

for ( int i = 0; i < iterations; i++ )
{
PageCache cache = createPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL );

// Touch all the pages
PagedFile pagedFile = cache.map( file( "a" ), filePageSize );
try ( PageCursor cursor = pagedFile.io( 0, PF_SHARED_LOCK ) )
{
for ( int j = 0; j < filePagesInTotal; j++ )
{
assertTrue( cursor.next() );
}
}

// We're now likely racing with the eviction thread
pagedFile.close();
cache.close();
refs.add( new WeakReference<>( cache ) );
createAndDirtyAndShutDownPageCache( refs, filePagesInTotal );

assertTrue( caughtExceptions.isEmpty() );
}
Expand All @@ -3605,13 +3586,17 @@ public void uncaughtException( Thread t, Throwable e )
// could possibly strongly reference the cache is any lingering background thread. If we do a couple of
// GCs, then we should observe that the WeakReference has been cleared by the garbage collector. If it
// hasn't, then something must be keeping it alive, even though it has been closed.
int maxChecks = 100;
int maxChecks = 200;
//noinspection unused -- we use old-gen heap pollution, as well as System.gc(), to cause old-gen GCs
byte[] randomHeapPollution;
boolean passed;
do
{
System.gc();
Thread.sleep( 100 );
passed = true;
//noinspection UnusedAssignment -- dumping unused crap into the old-gen heap to provoke GCs
randomHeapPollution = new byte[(int) ByteUnit.mebiBytes( 2 )];

for ( WeakReference<PageCache> ref : refs )
{
Expand Down Expand Up @@ -3642,6 +3627,27 @@ public void uncaughtException( Thread t, Throwable e )
}
}

private void createAndDirtyAndShutDownPageCache( List<WeakReference<PageCache>> refs, int filePagesInTotal )
throws IOException
{
PageCache cache = createPageCache( fs, maxPages, pageCachePageSize, PageCacheTracer.NULL );

// Touch all the pages
PagedFile pagedFile = cache.map( file( "a" ), filePageSize );
try ( PageCursor cursor = pagedFile.io( 0, PF_SHARED_LOCK ) )
{
for ( int j = 0; j < filePagesInTotal; j++ )
{
assertTrue( cursor.next() );
}
}

// We're now likely racing with the eviction thread
pagedFile.close();
cache.close();
refs.add( new WeakReference<>( cache ) );
}

@Test( timeout = SHORT_TIMEOUT_MILLIS )
public void pagesMustReturnToFreelistIfSwapInThrows() throws IOException
{
Expand Down

0 comments on commit 044eeab

Please sign in to comment.