Skip to content

Commit

Permalink
Fixes broken concurrency in SchemaCache
Browse files Browse the repository at this point in the history
Previously there were only some data structures (maps/sets) that used implementations
which could handle concurrent access. Seemingly that was added ad-hoc.
Originally the state in this class was meant to not care about concurrency
and instead rely on external synchronization. However this was never fully
the case since there could be unguarded reads happening concurrently with updates
to these non-concurrency-aware data structures, which means that some existing
data could be observed as missing when other unrelated data items were removed
or added. This was indeed proven by ConcurrentCreateDropIndexIT in an upcoming
commit.
  • Loading branch information
tinwelint committed Sep 22, 2017
1 parent a5e27f5 commit f047eec
Showing 1 changed file with 8 additions and 10 deletions.
Expand Up @@ -19,7 +19,6 @@
*/
package org.neo4j.kernel.impl.api.store;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
Expand All @@ -29,8 +28,6 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

import org.neo4j.collection.primitive.Primitive;
import org.neo4j.collection.primitive.PrimitiveIntObjectMap;
import org.neo4j.helpers.collection.Iterators;
import org.neo4j.kernel.api.schema.LabelSchemaDescriptor;
import org.neo4j.kernel.api.schema.SchemaDescriptor;
Expand All @@ -43,6 +40,7 @@
import org.neo4j.storageengine.api.schema.SchemaRule;

import static java.util.Collections.emptyIterator;
import static java.util.Collections.newSetFromMap;

/**
* A cache of {@link SchemaRule schema rules} as well as enforcement of schema consistency.
Expand All @@ -55,16 +53,16 @@
*/
public class SchemaCache
{
private final Map<Long, IndexRule> indexRuleById = new HashMap<>();
private final Map<Long, ConstraintRule> constraintRuleById = new HashMap<>();
private final Set<ConstraintDescriptor> constraints = new HashSet<>();
private final Map<Long,IndexRule> indexRuleById = new ConcurrentHashMap<>();
private final Map<Long,ConstraintRule> constraintRuleById = new ConcurrentHashMap<>();
private final Set<ConstraintDescriptor> constraints = newSetFromMap( new ConcurrentHashMap<>() );

private final Map<SchemaDescriptor,IndexDescriptor> indexDescriptors = new HashMap<>();
private final PrimitiveIntObjectMap<Set<IndexDescriptor>> indexDescriptorsByLabel = Primitive.intObjectMap();
private final Map<SchemaDescriptor,IndexDescriptor> indexDescriptors = new ConcurrentHashMap<>();
private final Map<Integer,Set<IndexDescriptor>> indexDescriptorsByLabel = new ConcurrentHashMap<>();
private final ConstraintSemantics constraintSemantics;

private final Map<Class<?>,Object> dependantState = new ConcurrentHashMap<>();
private final PrimitiveIntObjectMap<List<IndexDescriptor>> indexByProperty = Primitive.intObjectMap();
private final Map<Integer,List<IndexDescriptor>> indexByProperty = new ConcurrentHashMap<>();

public SchemaCache( ConstraintSemantics constraintSemantics, Iterable<SchemaRule> initialRules )
{
Expand Down Expand Up @@ -204,7 +202,7 @@ public void removeSchemaRule( long id )
if ( constraintRuleById.containsKey( id ) )
{
ConstraintRule rule = constraintRuleById.remove( id );
constraints.remove( constraintSemantics.readConstraint( rule ) );
constraints.remove( rule.getConstraintDescriptor() );
}
else if ( indexRuleById.containsKey( id ) )
{
Expand Down

0 comments on commit f047eec

Please sign in to comment.