Skip to content

Commit

Permalink
Ensure that IndexProxy returns the correct config
Browse files Browse the repository at this point in the history
When performing recovery of a constraint index, the IndexProxy would
contain an IndexConfiguration object that said that it was a normal
index rather than a constraint index. This was because we did not want
the index to have to go through a tentative phase, but instead go to the
online state immediately.

This issue is solved by separating the configuration of the index from
whether it should flip through a tentative state or not.

There was nothing in the code that depended on this IndexConfiguration
object being correct after recovery, but it is possible that such code
could be written in the future...
  • Loading branch information
thobe authored and chrisvest committed Aug 18, 2016
1 parent 8e61557 commit 55b6782
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 16 deletions.
Expand Up @@ -59,15 +59,15 @@ public IndexProxyCreator( IndexSamplingConfig samplingConfig,
public IndexProxy createPopulatingIndexProxy( final long ruleId, public IndexProxy createPopulatingIndexProxy( final long ruleId,
final IndexDescriptor descriptor, final IndexDescriptor descriptor,
final SchemaIndexProvider.Descriptor providerDescriptor, final SchemaIndexProvider.Descriptor providerDescriptor,
final boolean constraint, IndexConfiguration config,
final boolean flipToTentative,
final IndexingService.Monitor monitor, final IndexingService.Monitor monitor,
final IndexPopulationJob populationJob ) throws IOException final IndexPopulationJob populationJob ) throws IOException
{ {
final FlippableIndexProxy flipper = new FlippableIndexProxy(); final FlippableIndexProxy flipper = new FlippableIndexProxy();


// TODO: This is here because there is a circular dependency from PopulatingIndexProxy to FlippableIndexProxy // TODO: This is here because there is a circular dependency from PopulatingIndexProxy to FlippableIndexProxy
final String indexUserDescription = indexUserDescription( descriptor, providerDescriptor ); final String indexUserDescription = indexUserDescription( descriptor, providerDescriptor );
final IndexConfiguration config = IndexConfiguration.of( constraint );
IndexPopulator populator = populatorFromProvider( providerDescriptor, ruleId, descriptor, config, IndexPopulator populator = populatorFromProvider( providerDescriptor, ruleId, descriptor, config,
samplingConfig ); samplingConfig );


Expand Down Expand Up @@ -96,7 +96,7 @@ public IndexProxy createPopulatingIndexProxy( final long ruleId,
descriptor, config, onlineAccessorFromProvider( providerDescriptor, ruleId, descriptor, config, onlineAccessorFromProvider( providerDescriptor, ruleId,
config, samplingConfig ), storeView, providerDescriptor, true config, samplingConfig ), storeView, providerDescriptor, true
); );
if ( constraint ) if ( flipToTentative )
{ {
return new TentativeConstraintIndexProxy( flipper, onlineProxy ); return new TentativeConstraintIndexProxy( flipper, onlineProxy );
} }
Expand Down
Expand Up @@ -41,6 +41,7 @@
import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
import org.neo4j.kernel.api.exceptions.index.IndexPopulationFailedKernelException; import org.neo4j.kernel.api.exceptions.index.IndexPopulationFailedKernelException;
import org.neo4j.kernel.api.exceptions.schema.ConstraintVerificationFailedKernelException; import org.neo4j.kernel.api.exceptions.schema.ConstraintVerificationFailedKernelException;
import org.neo4j.kernel.api.index.IndexConfiguration;
import org.neo4j.kernel.api.index.IndexDescriptor; import org.neo4j.kernel.api.index.IndexDescriptor;
import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.api.index.IndexUpdater;
import org.neo4j.kernel.api.index.InternalIndexState; import org.neo4j.kernel.api.index.InternalIndexState;
Expand Down Expand Up @@ -210,8 +211,18 @@ public void init()
break; break;
case POPULATING: case POPULATING:
// The database was shut down during population, or a crash has occurred, or some other sad thing. // The database was shut down during population, or a crash has occurred, or some other sad thing.
indexProxy = indexProxyCreator if ( constraint && indexRule.getOwningConstraint() == null )
.createRecoveringIndexProxy( descriptor, providerDescriptor, constraint ); {
// don't bother rebuilding if we are going to throw the index away anyhow
indexProxy = indexProxyCreator.createFailedIndexProxy(
indexId, descriptor, providerDescriptor, constraint,
failure( "Constraint for index was not committed." ) );
}
else
{
indexProxy = indexProxyCreator
.createRecoveringIndexProxy( descriptor, providerDescriptor, constraint );
}
break; break;
case FAILED: case FAILED:
IndexPopulationFailure failure = failure( provider.getPopulationFailure( indexId ) ); IndexPopulationFailure failure = failure( provider.getPopulationFailure( indexId ) );
Expand Down Expand Up @@ -269,17 +280,16 @@ public void start() throws Exception
for ( Map.Entry<Long,RebuildingIndexDescriptor> entry : rebuildingDescriptors.entrySet() ) for ( Map.Entry<Long,RebuildingIndexDescriptor> entry : rebuildingDescriptors.entrySet() )
{ {
long indexId = entry.getKey(); long indexId = entry.getKey();
RebuildingIndexDescriptor descriptors = entry.getValue(); RebuildingIndexDescriptor descriptor = entry.getValue();


/*
* Passing in "false" for unique here may seem surprising, and.. well, yes, it is, I was surprised too.
* However, it is actually perfectly safe, because whenever we have constraint indexes here, they will
* be in a state where they didn't finish populating, and despite the fact that we re-create them here,
* they will get dropped as soon as recovery is completed by the constraint system.
*/
IndexProxy proxy = indexProxyCreator.createPopulatingIndexProxy( IndexProxy proxy = indexProxyCreator.createPopulatingIndexProxy(
indexId, descriptors.getIndexDescriptor(), descriptors.getProviderDescriptor(), false, indexId,
monitor, populationJob ); descriptor.getIndexDescriptor(),
descriptor.getProviderDescriptor(),
descriptor.getConfiguration(),
false, // never pass through a tentative online state during recovery
monitor,
populationJob );
proxy.start(); proxy.start();
indexMap.putIndexProxy( indexId, proxy ); indexMap.putIndexProxy( indexId, proxy );
} }
Expand Down Expand Up @@ -524,7 +534,8 @@ public void createIndexes( IndexRule... rules ) throws IOException
{ {
populationJob = populationJob == null ? newIndexPopulationJob() : populationJob; populationJob = populationJob == null ? newIndexPopulationJob() : populationJob;
index = indexProxyCreator.createPopulatingIndexProxy( index = indexProxyCreator.createPopulatingIndexProxy(
ruleId, descriptor, providerDescriptor, constraint, monitor, populationJob ); ruleId, descriptor, providerDescriptor, IndexConfiguration.of( constraint ), constraint,
monitor, populationJob );
index.start(); index.start();
} }
else else
Expand Down

0 comments on commit 55b6782

Please sign in to comment.