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 committed Aug 18, 2016
1 parent f0c2348 commit f5dbb46
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 15 deletions.
Expand Up @@ -31,7 +31,6 @@
import org.neo4j.kernel.impl.api.UpdateableSchemaState; import org.neo4j.kernel.impl.api.UpdateableSchemaState;
import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig;
import org.neo4j.kernel.impl.util.JobScheduler; import org.neo4j.kernel.impl.util.JobScheduler;
import org.neo4j.logging.Log;
import org.neo4j.logging.LogProvider; import org.neo4j.logging.LogProvider;


import static java.lang.String.format; import static java.lang.String.format;
Expand Down Expand Up @@ -67,14 +66,14 @@ public IndexProxySetup( 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, final IndexConfiguration config,
final boolean flipToTentative,
final IndexingService.Monitor monitor ) throws IOException final IndexingService.Monitor monitor ) 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 = new IndexConfiguration( constraint );
final IndexPopulator populator = populatorFromProvider( providerDescriptor, ruleId, descriptor, config, final IndexPopulator populator = populatorFromProvider( providerDescriptor, ruleId, descriptor, config,
samplingConfig ); samplingConfig );


Expand Down Expand Up @@ -103,7 +102,7 @@ public IndexProxy create() throws Exception
OnlineIndexProxy onlineProxy = new OnlineIndexProxy( OnlineIndexProxy onlineProxy = new OnlineIndexProxy(
descriptor, config, onlineAccessorFromProvider( providerDescriptor, ruleId, descriptor, config, onlineAccessorFromProvider( providerDescriptor, ruleId,
config, samplingConfig ), storeView, providerDescriptor ); config, samplingConfig ), storeView, providerDescriptor );
if ( constraint ) if ( flipToTentative )
{ {
return new TentativeConstraintIndexProxy( flipper, onlineProxy ); return new TentativeConstraintIndexProxy( flipper, onlineProxy );
} }
Expand Down
Expand Up @@ -43,6 +43,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.IndexEntryConflictException; import org.neo4j.kernel.api.index.IndexEntryConflictException;
import org.neo4j.kernel.api.index.IndexUpdater; import org.neo4j.kernel.api.index.IndexUpdater;
Expand Down Expand Up @@ -234,7 +235,16 @@ 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 = proxySetup.createRecoveringIndexProxy( descriptor, providerDescriptor, constraint ); if ( constraint && indexRule.getOwningConstraint() == null )
{
// don't bother rebuilding if we are going to throw the index away anyhow
indexProxy = proxySetup.createFailedIndexProxy( indexId, descriptor, providerDescriptor, constraint,
failure( "Constraint for index was not committed." ) );
}
else
{
indexProxy = proxySetup.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 @@ -293,16 +303,15 @@ public void accept( Long indexId, IndexProxy proxy )
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 = proxySetup.createPopulatingIndexProxy( IndexProxy proxy = proxySetup.createPopulatingIndexProxy(
indexId, descriptors.getIndexDescriptor(), descriptors.getProviderDescriptor(), false, monitor ); indexId,
descriptor.getIndexDescriptor(),
descriptor.getProviderDescriptor(),
descriptor.getConfiguration(),
false, // never pass through a tentative online state during recovery
monitor );
proxy.start(); proxy.start();
indexMap.putIndexProxy( indexId, proxy ); indexMap.putIndexProxy( indexId, proxy );
} }
Expand Down Expand Up @@ -438,7 +447,7 @@ public void createIndex( IndexRule rule )
try try
{ {
index = proxySetup.createPopulatingIndexProxy( index = proxySetup.createPopulatingIndexProxy(
ruleId, descriptor, providerDescriptor, constraint, monitor ); ruleId, descriptor, providerDescriptor, new IndexConfiguration( constraint ), constraint, monitor );
index.start(); index.start();
} }
catch ( IOException e ) catch ( IOException e )
Expand Down

0 comments on commit f5dbb46

Please sign in to comment.