From f5dbb460e5b13906cbb4b2b79e226a187a628ea6 Mon Sep 17 00:00:00 2001 From: Tobias Lindaaker Date: Tue, 16 Aug 2016 14:57:13 +0200 Subject: [PATCH] Ensure that IndexProxy returns the correct config 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... --- .../impl/api/index/IndexProxySetup.java | 7 ++--- .../impl/api/index/IndexingService.java | 31 ++++++++++++------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexProxySetup.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexProxySetup.java index 3bd9be5b9678b..e5b5269e50a20 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexProxySetup.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexProxySetup.java @@ -31,7 +31,6 @@ import org.neo4j.kernel.impl.api.UpdateableSchemaState; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; import org.neo4j.kernel.impl.util.JobScheduler; -import org.neo4j.logging.Log; import org.neo4j.logging.LogProvider; import static java.lang.String.format; @@ -67,14 +66,14 @@ public IndexProxySetup( IndexSamplingConfig samplingConfig, public IndexProxy createPopulatingIndexProxy( final long ruleId, final IndexDescriptor descriptor, final SchemaIndexProvider.Descriptor providerDescriptor, - final boolean constraint, + final IndexConfiguration config, + final boolean flipToTentative, final IndexingService.Monitor monitor ) throws IOException { final FlippableIndexProxy flipper = new FlippableIndexProxy(); // TODO: This is here because there is a circular dependency from PopulatingIndexProxy to FlippableIndexProxy final String indexUserDescription = indexUserDescription( descriptor, providerDescriptor ); - final IndexConfiguration config = new IndexConfiguration( constraint ); final IndexPopulator populator = populatorFromProvider( providerDescriptor, ruleId, descriptor, config, samplingConfig ); @@ -103,7 +102,7 @@ public IndexProxy create() throws Exception OnlineIndexProxy onlineProxy = new OnlineIndexProxy( descriptor, config, onlineAccessorFromProvider( providerDescriptor, ruleId, config, samplingConfig ), storeView, providerDescriptor ); - if ( constraint ) + if ( flipToTentative ) { return new TentativeConstraintIndexProxy( flipper, onlineProxy ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java index 82a0281ac5ce5..57008fa5840f0 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/index/IndexingService.java @@ -43,6 +43,7 @@ import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException; import org.neo4j.kernel.api.exceptions.index.IndexPopulationFailedKernelException; 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.IndexEntryConflictException; import org.neo4j.kernel.api.index.IndexUpdater; @@ -234,7 +235,16 @@ public void init() break; case POPULATING: // 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; case FAILED: IndexPopulationFailure failure = failure( provider.getPopulationFailure( indexId ) ); @@ -293,16 +303,15 @@ public void accept( Long indexId, IndexProxy proxy ) for ( Map.Entry entry : rebuildingDescriptors.entrySet() ) { long indexId = entry.getKey(); - RebuildingIndexDescriptor descriptors = 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. - */ + RebuildingIndexDescriptor descriptor = entry.getValue(); + 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(); indexMap.putIndexProxy( indexId, proxy ); } @@ -438,7 +447,7 @@ public void createIndex( IndexRule rule ) try { index = proxySetup.createPopulatingIndexProxy( - ruleId, descriptor, providerDescriptor, constraint, monitor ); + ruleId, descriptor, providerDescriptor, new IndexConfiguration( constraint ), constraint, monitor ); index.start(); } catch ( IOException e )