Skip to content

Commit

Permalink
CleanupJob - Improve logging, catch throwable, close on shutdown
Browse files Browse the repository at this point in the history
- Log CleanupJob registration, start, finish, close and failure
- CleanupJob catch Throwable and not only Exception
- Close CleanupJobs during GroupingRecoveryCleanupWorkCollector shutdown
- Fail if CleanupJobs exist during init
- BackupIT#backupMultipleSchemaIndexes
  • Loading branch information
burqen committed Jun 21, 2018
1 parent 81b1df8 commit f406700
Show file tree
Hide file tree
Showing 15 changed files with 461 additions and 113 deletions.
Expand Up @@ -40,7 +40,7 @@ public interface CleanupJob extends Runnable
/**
* @return Cause of failure if {@link #hasFailed()} or {@code null} if job has not failed.
*/
Exception getCause();
Throwable getCause();

/**
* Mark this job as closed and cleanup all it's resources.
Expand Down Expand Up @@ -70,7 +70,7 @@ public boolean hasFailed()
}

@Override
public Exception getCause()
public Throwable getCause()
{
return null;
}
Expand Down
Expand Up @@ -69,8 +69,9 @@ class CrashGenerationCleaner

public void clean() throws IOException
{
assert unstableGeneration > stableGeneration;
assert unstableGeneration - stableGeneration > 1;
monitor.cleanupStarted();
assert unstableGeneration > stableGeneration : unexpectedGenerations();
assert unstableGeneration - stableGeneration > 1 : unexpectedGenerations();

long startTime = currentTimeMillis();
int threads = availableProcessors;
Expand Down Expand Up @@ -238,4 +239,9 @@ private void cleanCrashedGSP( PageCursor cursor, int gspOffset, AtomicInteger cl
cleanedPointers.incrementAndGet();
}
}

private String unexpectedGenerations( )
{
return "Unexpected generations, stableGeneration=" + stableGeneration + ", unstableGeneration=" + unstableGeneration;
}
}
Expand Up @@ -142,11 +142,31 @@ public void noStoreFile()
{ // no-op
}

@Override
public void cleanupRegistered()
{ // no-op
}

@Override
public void cleanupStarted()
{ // no-op
}

@Override
public void cleanupFinished( long numberOfPagesVisited, long numberOfCleanedCrashPointers, long durationMillis )
{ // no-op
}

@Override
public void cleanupClosed()
{ // no-op
}

@Override
public void cleanupFailed( Throwable throwable )
{ // no-op
}

@Override
public void startupState( boolean clean )
{ // no-op
Expand All @@ -164,6 +184,16 @@ public void startupState( boolean clean )
*/
void noStoreFile();

/**
* Called after cleanup job has been created
*/
void cleanupRegistered();

/**
* Called after cleanup job has been started
*/
void cleanupStarted();

/**
* Called after recovery has completed and cleaning has been done.
*
Expand All @@ -173,6 +203,17 @@ public void startupState( boolean clean )
*/
void cleanupFinished( long numberOfPagesVisited, long numberOfCleanedCrashPointers, long durationMillis );

/**
* Called when cleanup job is closed and lock is released
*/
void cleanupClosed();

/**
* Called when cleanup job catches a throwable
* @param throwable cause of failure
*/
void cleanupFailed( Throwable throwable );

/**
* Report tree state on startup.
*
Expand Down Expand Up @@ -1006,6 +1047,7 @@ private CleanupJob createCleanupJob( RecoveryCleanupWorkCollector recoveryCleanu
else
{
lock.cleanerLock();
monitor.cleanupRegistered();

long generation = this.generation;
long stableGeneration = stableGeneration( generation );
Expand All @@ -1015,7 +1057,7 @@ private CleanupJob createCleanupJob( RecoveryCleanupWorkCollector recoveryCleanu
CrashGenerationCleaner crashGenerationCleaner =
new CrashGenerationCleaner( pagedFile, bTreeNode, IdSpace.MIN_TREE_NODE_ID, highTreeNodeId,
stableGeneration, unstableGeneration, monitor );
GBPTreeCleanupJob cleanupJob = new GBPTreeCleanupJob( crashGenerationCleaner, lock );
GBPTreeCleanupJob cleanupJob = new GBPTreeCleanupJob( crashGenerationCleaner, lock, monitor, indexFile );
recoveryCleanupWorkCollector.add( cleanupJob );
return cleanupJob;
}
Expand Down
Expand Up @@ -19,21 +19,30 @@
*/
package org.neo4j.index.internal.gbptree;

import java.io.File;
import java.util.StringJoiner;

class GBPTreeCleanupJob implements CleanupJob
{
private final CrashGenerationCleaner crashGenerationCleaner;
private final GBPTreeLock gbpTreeLock;
private final GBPTree.Monitor monitor;
private final File indexFile;
private volatile boolean needed;
private volatile Exception failure;
private volatile Throwable failure;

/**
* @param crashGenerationCleaner {@link CrashGenerationCleaner} to use for cleaning.
* @param gbpTreeLock {@link GBPTreeLock} to be released when job has either successfully finished or failed.
* @param monitor {@link GBPTree.Monitor} to report to
* @param indexFile Target file
*/
GBPTreeCleanupJob( CrashGenerationCleaner crashGenerationCleaner, GBPTreeLock gbpTreeLock )
GBPTreeCleanupJob( CrashGenerationCleaner crashGenerationCleaner, GBPTreeLock gbpTreeLock, GBPTree.Monitor monitor, File indexFile )
{
this.crashGenerationCleaner = crashGenerationCleaner;
this.gbpTreeLock = gbpTreeLock;
this.monitor = monitor;
this.indexFile = indexFile;
this.needed = true;

}
Expand All @@ -51,7 +60,7 @@ public boolean hasFailed()
}

@Override
public Exception getCause()
public Throwable getCause()
{
return failure;
}
Expand All @@ -60,6 +69,7 @@ public Exception getCause()
public void close()
{
gbpTreeLock.cleanerUnlock();
monitor.cleanupClosed();
}

@Override
Expand All @@ -70,9 +80,20 @@ public void run()
crashGenerationCleaner.clean();
needed = false;
}
catch ( Exception e )
catch ( Throwable e )
{
monitor.cleanupFailed( e );
failure = e;
}
}

@Override
public String toString()
{
StringJoiner joiner = new StringJoiner( ", ", "CleanupJob(", ")" );
joiner.add( "file=" + indexFile.getAbsolutePath() );
joiner.add( "needed=" + needed );
joiner.add( "failure=" + (failure == null ? null : failure.toString()) );
return joiner.toString();
}
}
Expand Up @@ -20,7 +20,9 @@
package org.neo4j.index.internal.gbptree;

import java.util.Queue;
import java.util.StringJoiner;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.function.Consumer;

import org.neo4j.kernel.lifecycle.LifecycleAdapter;
import org.neo4j.scheduler.JobScheduler;
Expand Down Expand Up @@ -51,7 +53,13 @@ public GroupingRecoveryCleanupWorkCollector( JobScheduler jobScheduler )
public void init()
{
started = false;
jobs.clear();
if ( !jobs.isEmpty() )
{
StringJoiner joiner = new StringJoiner( "\n ", "Did not expect there to be any cleanup jobs still here. Jobs[", "]" );
consumeAndCloseJobs( cj -> joiner.add( jobs.toString() ) );
throw new IllegalStateException( joiner.toString() );
}

}

@Override
Expand All @@ -71,6 +79,12 @@ public void start()
started = true;
}

@Override
public void shutdown()
{
consumeAndCloseJobs( cj -> {} );
}

private void scheduleJobs()
{
jobScheduler.schedule( recoveryCleanup, allJobs() );
Expand Down Expand Up @@ -110,4 +124,14 @@ private Runnable allJobs()
}
};
}

private void consumeAndCloseJobs( Consumer<CleanupJob> consumer )
{
CleanupJob job;
while ( (job = jobs.poll()) != null )
{
consumer.accept( job );
job.close();
}
}
}

0 comments on commit f406700

Please sign in to comment.