Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
klaren authored and MishaDemianenko committed Jul 20, 2018
1 parent b286cd3 commit 68d72bc
Show file tree
Hide file tree
Showing 12 changed files with 126 additions and 99 deletions.
Expand Up @@ -355,7 +355,8 @@ public void startupState( boolean clean )
* Whether or not this tree has been closed. Accessed and changed solely in * Whether or not this tree has been closed. Accessed and changed solely in
* {@link #close()} to be able to close tree multiple times gracefully. * {@link #close()} to be able to close tree multiple times gracefully.
*/ */
private boolean closed; @SuppressWarnings( "UnusedAssignment" )
private boolean closed = true;


/** /**
* True if tree is clean, false if dirty * True if tree is clean, false if dirty
Expand Down Expand Up @@ -510,7 +511,7 @@ private RuntimeException exitConstructor( Throwable throwable )
} }
catch ( IOException e ) catch ( IOException e )
{ {
Exceptions.chain( new UncheckedIOException( e ), throwable ); throwable = Exceptions.chain( new UncheckedIOException( e ), throwable );
} }


appendTreeInformation( throwable ); appendTreeInformation( throwable );
Expand Down
32 changes: 24 additions & 8 deletions community/io/src/main/java/org/neo4j/io/IOUtils.java
Expand Up @@ -20,9 +20,12 @@
package org.neo4j.io; package org.neo4j.io;


import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.reflect.Constructor; import java.lang.reflect.Constructor;
import java.util.Collection; import java.util.Collection;


import org.neo4j.helpers.Exceptions;

/** /**
* IO helper methods. * IO helper methods.
*/ */
Expand All @@ -45,6 +48,26 @@ public static <T extends AutoCloseable> void closeAll( Collection<T> closeables
closeAll( closeables.toArray( new AutoCloseable[0] ) ); closeAll( closeables.toArray( new AutoCloseable[0] ) );
} }


/**
* Close all the provided {@link AutoCloseable closeables}, chaining exceptions, if any, into a single {@link UncheckedIOException}.
*
* @param closeables to call close on.
* @param <T> the type of closeable.
* @throws UncheckedIOException if any exception is thrown from any of the {@code closeables}.
*/
@SafeVarargs
public static <T extends AutoCloseable> void closeAllUnchecked( T... closeables ) throws UncheckedIOException
{
try
{
closeAll( closeables );
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
}

/** /**
* Closes given {@link Collection collection} of {@link AutoCloseable closeables} ignoring all exceptions. * Closes given {@link Collection collection} of {@link AutoCloseable closeables} ignoring all exceptions.
* *
Expand Down Expand Up @@ -123,14 +146,7 @@ public static <T extends AutoCloseable, E extends Throwable> void closeAll( Clas
} }
catch ( Throwable t ) catch ( Throwable t )
{ {
if ( closeThrowable == null ) closeThrowable = Exceptions.chain( closeThrowable, t );
{
closeThrowable = t;
}
else
{
closeThrowable.addSuppressed( t );
}
} }
} }
} }
Expand Down
Expand Up @@ -39,6 +39,17 @@ public class IndexEntryConflictException extends Exception
private final long addedNodeId; private final long addedNodeId;
private final long existingNodeId; private final long existingNodeId;


/**
* Make IOUtils happy
*/
public IndexEntryConflictException( String message, Throwable cause )
{
super( message, cause );
propertyValues = null;
addedNodeId = -1;
existingNodeId = -1;
}

public IndexEntryConflictException( long existingNodeId, long addedNodeId, Value... propertyValue ) public IndexEntryConflictException( long existingNodeId, long addedNodeId, Value... propertyValue )
{ {
this( existingNodeId, addedNodeId, ValueTuple.of( propertyValue ) ); this( existingNodeId, addedNodeId, ValueTuple.of( propertyValue ) );
Expand Down
Expand Up @@ -44,7 +44,6 @@
import org.neo4j.kernel.api.labelscan.LabelScanStore; import org.neo4j.kernel.api.labelscan.LabelScanStore;
import org.neo4j.kernel.api.labelscan.LabelScanWriter; import org.neo4j.kernel.api.labelscan.LabelScanWriter;
import org.neo4j.kernel.impl.api.scan.FullStoreChangeStream; import org.neo4j.kernel.impl.api.scan.FullStoreChangeStream;
import org.neo4j.kernel.impl.store.UnderlyingStorageException;
import org.neo4j.kernel.monitoring.Monitors; import org.neo4j.kernel.monitoring.Monitors;
import org.neo4j.storageengine.api.schema.LabelScanReader; import org.neo4j.storageengine.api.schema.LabelScanReader;


Expand Down Expand Up @@ -247,19 +246,11 @@ public LabelScanWriter newWriter()
* and non-clean shutdown will be applied again on next startup. * and non-clean shutdown will be applied again on next startup.
* *
* @param limiter {@link IOLimiter}. * @param limiter {@link IOLimiter}.
* @throws UnderlyingStorageException on failure writing changes to {@link PageCache}.
*/ */
@Override @Override
public void force( IOLimiter limiter ) throws UnderlyingStorageException public void force( IOLimiter limiter )
{ {
try index.checkpoint( limiter );
{
index.checkpoint( limiter );
}
catch ( UncheckedIOException e )
{
throw new UnderlyingStorageException( e );
}
} }


@Override @Override
Expand Down
Expand Up @@ -19,7 +19,6 @@
*/ */
package org.neo4j.kernel.impl.index.schema; package org.neo4j.kernel.impl.index.schema;


import java.io.Closeable;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException; import java.io.UncheckedIOException;
Expand All @@ -28,6 +27,7 @@
import org.neo4j.index.internal.gbptree.GBPTree; import org.neo4j.index.internal.gbptree.GBPTree;
import org.neo4j.index.internal.gbptree.Layout; import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.index.internal.gbptree.RecoveryCleanupWorkCollector; import org.neo4j.index.internal.gbptree.RecoveryCleanupWorkCollector;
import org.neo4j.io.IOUtils;
import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.io.pagecache.PageCache; import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PageCursor;
Expand Down Expand Up @@ -85,23 +85,8 @@ private void ensureDirectoryExist()


void closeTree() void closeTree()
{ {
tree = closeIfPresent( tree ); IOUtils.closeAllUnchecked( tree );
} tree = null;

private static <T extends Closeable> T closeIfPresent( T closeable )
{
if ( closeable != null )
{
try
{
closeable.close();
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
}
return null;
} }


void assertOpen() void assertOpen()
Expand Down
Expand Up @@ -19,10 +19,8 @@
*/ */
package org.neo4j.kernel.impl.index.schema; package org.neo4j.kernel.impl.index.schema;


import java.io.IOException;
import java.io.UncheckedIOException;

import org.neo4j.index.internal.gbptree.Writer; import org.neo4j.index.internal.gbptree.Writer;
import org.neo4j.io.IOUtils;
import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException; import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
import org.neo4j.kernel.api.index.IndexEntryUpdate; import org.neo4j.kernel.api.index.IndexEntryUpdate;
import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.api.index.IndexUpdater;
Expand Down Expand Up @@ -66,14 +64,7 @@ public void process( IndexEntryUpdate<?> update ) throws IndexEntryConflictExcep
public void close() public void close()
{ {
closed = true; closed = true;
try IOUtils.closeAllUnchecked( writer );
{
writer.close();
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
} }


private void assertOpen() private void assertOpen()
Expand Down
Expand Up @@ -94,7 +94,38 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
BridgingIndexProgressor multiProgressor = new BridgingIndexProgressor( cursor, BridgingIndexProgressor multiProgressor = new BridgingIndexProgressor( cursor,
descriptor.schema().getPropertyIds() ); descriptor.schema().getPropertyIds() );
cursor.initialize( descriptor, multiProgressor, predicates ); cursor.initialize( descriptor, multiProgressor, predicates );
instanceSelector.forAll( reader -> reader.query( multiProgressor, indexOrder, predicates ) ); try
{
instanceSelector.forAll( reader ->
{
try
{
reader.query( multiProgressor, indexOrder, predicates );
}
catch ( IndexNotApplicableKernelException e )
{
throw new InnerException( e );
}
} );
}
catch ( InnerException e )
{
throw e.getCause();
}
}
}

private static final class InnerException extends RuntimeException
{
private InnerException( IndexNotApplicableKernelException e )
{
super( e );
}

@Override
public synchronized IndexNotApplicableKernelException getCause()
{
return (IndexNotApplicableKernelException) super.getCause();
} }
} }


Expand Down
Expand Up @@ -124,7 +124,7 @@ <R,E extends Exception> Iterable<R> transform( ThrowingFunction<T,R,E> converter
/** /**
* Convenience method for doing something to all instances, even those that haven't already been instantiated. * Convenience method for doing something to all instances, even those that haven't already been instantiated.
* *
* @param consumer {@link ThrowingConsumer} which performs some action on an instance. * @param consumer {@link Consumer} which performs some action on an instance.
*/ */
void forAll( Consumer<T> consumer ) void forAll( Consumer<T> consumer )
{ {
Expand All @@ -142,7 +142,7 @@ void forAll( Consumer<T> consumer )
/** /**
* Perform a final action on instantiated instances and then closes this selector, preventing further instantiation. * Perform a final action on instantiated instances and then closes this selector, preventing further instantiation.
* *
* @param consumer {@link ThrowingConsumer} which performs some action on an instance. * @param consumer {@link Consumer} which performs some action on an instance.
*/ */
void close( Consumer<T> consumer ) void close( Consumer<T> consumer )
{ {
Expand Down

0 comments on commit 68d72bc

Please sign in to comment.