Skip to content

Commit

Permalink
Make CountsTracker rotation timeout robust against drift
Browse files Browse the repository at this point in the history
Hopefully this will also make the AbstractKeyValueStoreTest less flaky on very resource strained systems, but just to be sure I have also increased the timeout by a factor 10.
  • Loading branch information
chrisvest committed Jul 4, 2017
1 parent 6c950d2 commit 96c4623
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 31 deletions.
Expand Up @@ -21,7 +21,6 @@

import java.io.File;
import java.io.IOException;
import java.time.Clock;
import java.util.Optional;

import org.neo4j.io.fs.FileSystemAbstraction;
Expand All @@ -47,6 +46,7 @@
import org.neo4j.logging.LogProvider;
import org.neo4j.register.Register;
import org.neo4j.time.Clocks;
import org.neo4j.time.SystemNanoClock;

import static java.lang.String.format;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.counts_store_rotation_timeout;
Expand Down Expand Up @@ -87,11 +87,11 @@ public class CountsTracker extends AbstractKeyValueStore<CountsKey>
public CountsTracker( final LogProvider logProvider, FileSystemAbstraction fs, PageCache pages, Config config,
File baseFile )
{
this( logProvider, fs, pages, config, baseFile, Clocks.systemClock() );
this( logProvider, fs, pages, config, baseFile, Clocks.nanoClock() );
}

public CountsTracker( final LogProvider logProvider, FileSystemAbstraction fs, PageCache pages, Config config,
File baseFile, Clock clock )
File baseFile, SystemNanoClock clock )
{
super( fs, pages, baseFile, new RotationMonitor()
{
Expand Down
Expand Up @@ -19,44 +19,48 @@
*/
package org.neo4j.kernel.impl.store.kvstore;

import java.time.Clock;
import java.util.concurrent.TimeUnit;

import org.neo4j.time.SystemNanoClock;

public class RotationTimerFactory
{
private Clock clock;
private SystemNanoClock clock;
private long timeoutMillis;

public RotationTimerFactory( Clock clock, long timeoutMillis )
public RotationTimerFactory( SystemNanoClock clock, long timeoutMillis )
{
this.clock = clock;
this.timeoutMillis = timeoutMillis;
}

public RotationTimer createTimer()
{
long startTime = clock.millis();
return new RotationTimer( startTime, startTime + timeoutMillis );
long startTimeNanos = clock.nanos();
long timeoutNanos = TimeUnit.MILLISECONDS.toNanos( timeoutMillis );
return new RotationTimer( startTimeNanos, startTimeNanos + timeoutNanos );
}

class RotationTimer
{
private long startTime;
private long timeoutTime;
private long startTimeNanos;
private long deadlineNanos;

RotationTimer( long startTime, long timeoutTime )
RotationTimer( long startTimeNanos, long deadlineNanos )
{
this.startTime = startTime;
this.timeoutTime = timeoutTime;
this.startTimeNanos = startTimeNanos;
this.deadlineNanos = deadlineNanos;
}

public boolean isTimedOut()
{
return clock.millis() > timeoutTime;
return clock.nanos() > deadlineNanos;
}

public long getElapsedTimeMillis()
{
return clock.millis() - startTime;
long elapsedNanos = clock.nanos() - startTimeNanos;
return TimeUnit.NANOSECONDS.toMillis( elapsedNanos );
}

}
Expand Down
Expand Up @@ -24,7 +24,6 @@

import java.io.File;
import java.io.IOException;
import java.time.Clock;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
import java.util.function.Predicate;
Expand All @@ -48,6 +47,7 @@
import org.neo4j.test.rule.concurrent.ThreadingRule;
import org.neo4j.time.Clocks;
import org.neo4j.time.FakeClock;
import org.neo4j.time.SystemNanoClock;

import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;
Expand Down Expand Up @@ -405,10 +405,10 @@ public void shouldNotEndUpInBrokenStateAfterRotationFailure() throws Exception

private CountsTracker newTracker()
{
return newTracker( Clocks.systemClock() );
return newTracker( Clocks.nanoClock() );
}

private CountsTracker newTracker( Clock clock )
private CountsTracker newTracker( SystemNanoClock clock )
{
return new CountsTracker( resourceManager.logProvider(), resourceManager.fileSystem(),
resourceManager.pageCache(), Config.empty(), resourceManager.testPath(), clock )
Expand Down
Expand Up @@ -23,6 +23,7 @@
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.RuleChain;
import org.junit.rules.Timeout;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -54,15 +55,17 @@

public class AbstractKeyValueStoreTest
{

private final ExpectedException expectedException = ExpectedException.none();
private final Resources resourceManager = new Resources( FILE_IN_EXISTING_DIRECTORY );
private final ThreadingRule threading = new ThreadingRule();
private final Timeout timeout = Timeout.builder()
.withTimeout( 20, TimeUnit.SECONDS )
.withLookingForStuckThread( true )
.build();

@Rule
public final ThreadingRule threading = new ThreadingRule();
@Rule
public final RuleChain ruleChain = RuleChain.outerRule( expectedException )
.around( resourceManager );
.around( resourceManager ).around( threading ).around( timeout );

private static final HeaderField<Long> TX_ID = new HeaderField<Long>()
{
Expand Down Expand Up @@ -403,11 +406,10 @@ public void shouldBlockRotationUntilRequestedTransactionsAreApplied() throws Exc
store.rotation.apply( 4L );
}

@Test( timeout = 2000 )
@Test
@Resources.Life( STARTED )
public void shouldFailRotationAfterTimeout() throws IOException
{

// GIVEN
final Store store = resourceManager.managed( createTestStore( 0 ) );

Expand Down Expand Up @@ -593,7 +595,7 @@ private Store( HeaderField<?>... headerFields )
private Store( long rotationTimeout, HeaderField<?>... headerFields )
{
super( resourceManager.fileSystem(), resourceManager.pageCache(), resourceManager.testPath(), null,
new RotationTimerFactory( Clocks.systemClock(), rotationTimeout ), 16, 16, headerFields );
new RotationTimerFactory( Clocks.nanoClock(), rotationTimeout ), 16, 16, headerFields );
this.headerFields = headerFields;
setEntryUpdaterInitializer( new DataInitializer<EntryUpdater<String>>()
{
Expand Down
Expand Up @@ -22,9 +22,6 @@
import org.junit.Assert;
import org.junit.Test;

import java.time.Clock;
import java.time.Instant;
import java.time.ZoneOffset;
import java.util.concurrent.TimeUnit;

import org.neo4j.time.Clocks;
Expand All @@ -36,10 +33,10 @@ public class RotationTimerFactoryTest
public void testTimer() throws Exception
{
// GIVEN
Clock fixedClock = Clock.fixed( Instant.ofEpochMilli( 10000 ), ZoneOffset.UTC );
FakeClock fakeClock = Clocks.fakeClock( 10000 , TimeUnit.MILLISECONDS );

// WHEN
RotationTimerFactory timerFactory = new RotationTimerFactory( fixedClock, 1000 );
RotationTimerFactory timerFactory = new RotationTimerFactory( fakeClock, 1000 );
RotationTimerFactory.RotationTimer timer = timerFactory.createTimer();
RotationTimerFactory.RotationTimer anotherTimer = timerFactory.createTimer();

Expand All @@ -49,7 +46,7 @@ public void testTimer() throws Exception
Assert.assertNotSame( "Factory should construct different timers", timer, anotherTimer );

// WHEN
FakeClock fakeClock = Clocks.fakeClock();
fakeClock = Clocks.fakeClock();
RotationTimerFactory fakeTimerFactory = new RotationTimerFactory( fakeClock, 1000 );
RotationTimerFactory.RotationTimer fakeTimer = fakeTimerFactory.createTimer();
fakeClock.forward( 1001, TimeUnit.MILLISECONDS );
Expand Down

0 comments on commit 96c4623

Please sign in to comment.