Skip to content

Commit

Permalink
Let update queue drain when dropping fulltextindex
Browse files Browse the repository at this point in the history
This is so that no lucene writers holds locks on files that we need to
delete.

This commit also contains cleanup after a rebase
  • Loading branch information
ragadeeshu committed Oct 5, 2017
1 parent b6bef90 commit ace5443
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 7 deletions.
Expand Up @@ -20,11 +20,13 @@
package org.neo4j.kernel.api.impl.fulltext; package org.neo4j.kernel.api.impl.fulltext;


import java.io.IOException; import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock;
Expand Down Expand Up @@ -108,9 +110,20 @@ private boolean matchesConfiguration( WritableFulltext index ) throws IOExceptio
* needs to recover after an unclean shut-down, or a configuration change. * needs to recover after an unclean shut-down, or a configuration change.
* @throws IOException If it was not possible to wait for the population to finish, for some reason. * @throws IOException If it was not possible to wait for the population to finish, for some reason.
*/ */
public void awaitPopulation() throws Exception public void awaitPopulation()
{ {
applier.writeBarrier().awaitCompletion(); try
{
applier.writeBarrier().awaitCompletion();
}
catch ( ExecutionException e )
{
throw new AssertionError( "The writeBarrier operation should never throw an exception", e );
}
catch ( IOException e )
{
throw new UncheckedIOException( e );
}
} }


/** /**
Expand Down Expand Up @@ -246,6 +259,8 @@ void drop( String identifier, FulltextIndexType type ) throws IOException
configurationLock.writeLock().lock(); configurationLock.writeLock().lock();
try try
{ {
// Wait for the queue of updates to drain, before deleting an index.
awaitPopulation();
if ( type == FulltextIndexType.NODES ) if ( type == FulltextIndexType.NODES )
{ {
writableNodeIndices.remove( identifier ).drop(); writableNodeIndices.remove( identifier ).drop();
Expand Down
Expand Up @@ -746,7 +746,7 @@ public void shouldBeAbleToDropAndReaddIndex() throws Exception
} }


@Test @Test
public void concurrentUpdatesAndIndexChangesShouldREsultInValidState() throws Throwable public void concurrentUpdatesAndIndexChangesShouldResultInValidState() throws Throwable
{ {
try ( FulltextProvider provider = createProvider() ) try ( FulltextProvider provider = createProvider() )
{ {
Expand Down
Expand Up @@ -296,21 +296,21 @@ public void shouldNotBeAbleToFindNodesAfterRemovingIndex() throws Exception


// Verify it's indexed exactly once // Verify it's indexed exactly once
db.execute( "CALL db.fulltext.bloomAwaitPopulation" ).close(); db.execute( "CALL db.fulltext.bloomAwaitPopulation" ).close();
Result result = db.execute( String.format( NODES, "Jyllingevej", true, false ) ); Result result = db.execute( String.format( NODES, "\"Jyllingevej\"" ) );
assertTrue( result.hasNext() ); assertTrue( result.hasNext() );
assertEquals( nodeId, result.next().get( ENTITYID ) ); assertEquals( nodeId, result.next().get( ENTITYID ) );
assertFalse( result.hasNext() ); assertFalse( result.hasNext() );
db.execute( String.format( KEYS, "" ) ); db.execute( String.format( KEYS, "" ) );


db.execute( "CALL db.fulltext.bloomAwaitPopulation" ).close(); db.execute( "CALL db.fulltext.bloomAwaitPopulation" ).close();
// Verify it's nowhere to be found now // Verify it's nowhere to be found now
result = db.execute( String.format( NODES, "Jyllingevej", true, false ) ); result = db.execute( String.format( NODES, "\"Jyllingevej\"" ) );
assertFalse( result.hasNext() ); assertFalse( result.hasNext() );


db.shutdown(); db.shutdown();
db = getDb(); db = getDb();
// Should not be found after restart // Should not be found after restart
result = db.execute( String.format( NODES, "Jyllingevej", true, false ) ); result = db.execute( String.format( NODES, "\"Jyllingevej\"" ) );
assertFalse( result.hasNext() ); assertFalse( result.hasNext() );
} }


Expand Down Expand Up @@ -562,7 +562,7 @@ public void indexedPropertiesShouldBeSetByProcedure() throws Exception


try ( Transaction ignore = db.beginTx() ) try ( Transaction ignore = db.beginTx() )
{ {
try ( Result result = db.execute( String.format( NODES, "hello", false, false ) ) ) try ( Result result = db.execute( String.format( NODES_ADVANCED, "\"hello\"", false, false ) ) )
{ {
assertThat( Iterators.count( result ), is( 1L ) ); assertThat( Iterators.count( result ), is( 1L ) );
} }
Expand Down

0 comments on commit ace5443

Please sign in to comment.