Skip to content

Commit

Permalink
Simplified StoreReadLayer
Browse files Browse the repository at this point in the history
  • Loading branch information
fickludd committed Feb 24, 2017
1 parent 03459b3 commit 79550ea
Show file tree
Hide file tree
Showing 12 changed files with 51 additions and 87 deletions.
Expand Up @@ -23,7 +23,6 @@

import java.util.Iterator;
import java.util.function.Function;
import java.util.function.Predicate;

import org.neo4j.kernel.api.schema.NodePropertyDescriptor;
import org.neo4j.kernel.api.schema.RelationshipPropertyDescriptor;
Expand Down Expand Up @@ -243,12 +242,12 @@ public Long indexGetOwningUniquenessConstraintId( KernelStatement state, NewInde
}

@Override
public long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
public long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index )
throws SchemaRuleNotFoundException
{
acquireSharedSchemaLock( state );
state.assertOpen();
return schemaReadDelegate.indexGetCommittedId( state, index, filter );
return schemaReadDelegate.indexGetCommittedId( state, index );
}

@Override
Expand Down
Expand Up @@ -1186,10 +1186,10 @@ public Long indexGetOwningUniquenessConstraintId( KernelStatement state, NewInde
}

@Override
public long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
public long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index )
throws SchemaRuleNotFoundException
{
return storeLayer.indexGetCommittedId( index, filter );
return storeLayer.indexGetCommittedId( index );
}

@Override
Expand Down
Expand Up @@ -20,7 +20,6 @@
package org.neo4j.kernel.impl.api.operations;

import java.util.Iterator;
import java.util.function.Predicate;

import org.neo4j.kernel.api.schema.NodePropertyDescriptor;
import org.neo4j.kernel.api.schema.RelationshipPropertyDescriptor;
Expand Down Expand Up @@ -129,6 +128,5 @@ Iterator<RelationshipPropertyConstraint> constraintsGetForRelationshipTypeAndPro
* Get the index id (the id or the schema rule record) for a committed index
* - throws exception for indexes that aren't committed.
*/
long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
throws SchemaRuleNotFoundException;
long indexGetCommittedId( KernelStatement state, NewIndexDescriptor index ) throws SchemaRuleNotFoundException;
}
Expand Up @@ -96,7 +96,7 @@ public long createUniquenessConstraintIndex( KernelStatement state, SchemaReadOp
Client locks = state.locks().pessimistic();
try
{
long indexId = schema.indexGetCommittedId( state, index, NewIndexDescriptor.Filter.UNIQUE );
long indexId = schema.indexGetCommittedId( state, index );

// Release the SCHEMA WRITE lock during index population.
// At this point the integrity of the constraint to be created was checked
Expand Down
Expand Up @@ -218,7 +218,7 @@ public Iterator<NewIndexDescriptor> uniquenessIndexesGetAll()
@Override
public Long indexGetOwningUniquenessConstraintId( NewIndexDescriptor index ) throws SchemaRuleNotFoundException
{
IndexRule rule = indexRule( index, NewIndexDescriptor.Filter.ANY );
IndexRule rule = indexRule( index );
if ( rule != null )
{
return rule.getOwningConstraint();
Expand All @@ -227,10 +227,10 @@ public Long indexGetOwningUniquenessConstraintId( NewIndexDescriptor index ) thr
}

@Override
public long indexGetCommittedId( NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
public long indexGetCommittedId( NewIndexDescriptor index )
throws SchemaRuleNotFoundException
{
IndexRule rule = indexRule( index, filter );
IndexRule rule = indexRule( index );
if ( rule == null )
{
throw new SchemaRuleNotFoundException( SchemaRule.Kind.INDEX_RULE, index.schema() );
Expand Down Expand Up @@ -560,17 +560,17 @@ public void degrees( StorageStatement statement, NodeItem nodeItem, DegreeVisito
}
}

private IndexRule indexRule( NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
private IndexRule indexRule( NewIndexDescriptor index )
{
for ( IndexRule rule : schemaCache.indexRules() )
{
if ( filter.test( rule.getIndexDescriptor() ) && rule.schema().equals( index.schema() ) )
if ( rule.getIndexDescriptor().equals( index ) )
{
return rule;
}
}

return schemaStorage.indexGetForSchema( index.schema(), filter );
return schemaStorage.indexGetForSchema( index );
}

@Override
Expand Down
Expand Up @@ -30,10 +30,10 @@
import org.neo4j.kernel.api.exceptions.schema.SchemaRuleNotFoundException;
import org.neo4j.kernel.api.index.SchemaIndexProvider;
import org.neo4j.kernel.api.properties.DefinedProperty;
import org.neo4j.kernel.api.schema_new.SchemaDescriptorFactory;
import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptorFactory;
import org.neo4j.kernel.api.schema_new.index.IndexBoundary;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptorFactory;
import org.neo4j.kernel.impl.api.index.SchemaIndexProviderMap;
import org.neo4j.kernel.impl.constraints.ConstraintSemantics;
import org.neo4j.kernel.impl.store.SchemaStorage;
Expand All @@ -42,8 +42,6 @@
import org.neo4j.storageengine.api.StorageProperty;
import org.neo4j.storageengine.api.txstate.TxStateVisitor;

import static org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor.Type.UNIQUE;

public class TransactionToRecordStateVisitor extends TxStateVisitor.Adapter
{
private boolean clearSchemaState;
Expand Down Expand Up @@ -192,10 +190,7 @@ public void visitAddedIndex( NewIndexDescriptor index )
@Override
public void visitRemovedIndex( NewIndexDescriptor index )
{
NewIndexDescriptor.Filter filter = index.type() == UNIQUE ?
NewIndexDescriptor.Filter.UNIQUE
: NewIndexDescriptor.Filter.GENERAL;
IndexRule rule = schemaStorage.indexGetForSchema( index.schema(), filter );
IndexRule rule = schemaStorage.indexGetForSchema( index );
recordState.dropSchemaRule( rule );
}

Expand All @@ -207,7 +202,7 @@ public void visitAddedUniquePropertyConstraint( UniquenessConstraint element )
int propertyKeyId = element.descriptor().getPropertyKeyId();

IndexRule indexRule = schemaStorage.indexGetForSchema(
SchemaDescriptorFactory.forLabel( element.label(), propertyKeyId ), NewIndexDescriptor.Filter.UNIQUE );
NewIndexDescriptorFactory.uniqueForLabel( element.label(), propertyKeyId ) );
recordState.createSchemaRule(
constraintSemantics.writeUniquePropertyConstraint(
constraintId, element.descriptor(), indexRule.getId() ) );
Expand Down
Expand Up @@ -48,40 +48,27 @@ public SchemaStorage( RecordStore<DynamicRecord> schemaStore )
}

/**
* Find the IndexRule, of any kind, for the given SchemaDescriptor.
* Find the IndexRule that matches the given NewIndexDescriptor.
*
* @return the matching IndexRule, or null if no matching IndexRule was found
* @throws IllegalStateException if more than one matching rule.
* @param descriptor the target NewIndexDescriptor
*/
public IndexRule indexGetForSchema( SchemaDescriptor schemaDescriptor )
public IndexRule indexGetForSchema( final NewIndexDescriptor descriptor )
{
return indexGetForSchema( schemaDescriptor, NewIndexDescriptor.Filter.ANY );
}

/**
* Find the IndexRule that matches both the given SchemaDescriptor and passed the filter.
*
* @return the matching IndexRule, or null if no matching IndexRule was found
* @throws IllegalStateException if more than one matching rule.
*/
public IndexRule indexGetForSchema( final SchemaDescriptor descriptor, NewIndexDescriptor.Filter filter )
{
Iterator<IndexRule> rules = loadAllSchemaRules( SchemaDescriptor.equalTo( descriptor ), IndexRule.class, false );
Iterator<IndexRule> rules = loadAllSchemaRules( descriptor::isSame, IndexRule.class, false );

IndexRule foundRule = null;

while ( rules.hasNext() )
{
IndexRule candidate = rules.next();
if ( filter.test( candidate.getIndexDescriptor() ) )
if ( foundRule != null )
{
if ( foundRule != null )
{
throw new IllegalStateException( String.format(
"Found more than one matching index rule, %s and %s", foundRule, candidate ) );
}
foundRule = candidate;
throw new IllegalStateException( String.format(
"Found more than one matching index rule, %s and %s", foundRule, candidate ) );
}
foundRule = candidate;
}

return foundRule;
Expand Down
Expand Up @@ -89,11 +89,10 @@ Long indexGetOwningUniquenessConstraintId( NewIndexDescriptor index )

/**
* @param index {@link NewIndexDescriptor} to get schema rule id for.
* @param filter for type of index to match.
* @return schema rule id for matching index.
* @throws SchemaRuleNotFoundException if no such index exists in storage.
*/
long indexGetCommittedId( NewIndexDescriptor index, NewIndexDescriptor.Filter filter )
long indexGetCommittedId( NewIndexDescriptor index )
throws SchemaRuleNotFoundException;

/**
Expand Down
Expand Up @@ -35,15 +35,12 @@
import org.neo4j.kernel.api.exceptions.TransactionFailureException;
import org.neo4j.kernel.api.exceptions.index.IndexPopulationFailedKernelException;
import org.neo4j.kernel.api.exceptions.schema.ConstraintVerificationFailedKernelException;
import org.neo4j.kernel.api.schema.IndexDescriptor;
import org.neo4j.kernel.api.schema.IndexDescriptorFactory;
import org.neo4j.kernel.api.index.PreexistingIndexEntryConflictException;
import org.neo4j.kernel.api.index.PropertyAccessor;
import org.neo4j.kernel.api.proc.CallableProcedure;
import org.neo4j.kernel.api.proc.CallableUserAggregationFunction;
import org.neo4j.kernel.api.proc.CallableUserFunction;
import org.neo4j.kernel.api.schema_new.SchemaBoundary;
import org.neo4j.kernel.api.schema_new.index.IndexBoundary;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptorFactory;
import org.neo4j.kernel.api.security.SecurityContext;
Expand All @@ -65,7 +62,6 @@
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import static org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor.Filter.UNIQUE;
import static org.neo4j.kernel.impl.api.StatementOperationsTestHelper.mockedParts;
import static org.neo4j.kernel.impl.api.StatementOperationsTestHelper.mockedState;

Expand All @@ -86,7 +82,7 @@ public void shouldCreateIndexInAnotherTransaction() throws Exception
IndexingService indexingService = mock( IndexingService.class );
StubKernel kernel = new StubKernel();

when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index, UNIQUE ) )
when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index ) )
.thenReturn( 2468L );
IndexProxy indexProxy = mock( IndexProxy.class );
when( indexingService.getIndexProxy( 2468L ) ).thenReturn( indexProxy );
Expand All @@ -102,7 +98,7 @@ public void shouldCreateIndexInAnotherTransaction() throws Exception
assertEquals( 1, kernel.statements.size() );
verify( kernel.statements.get( 0 ).txState() ).indexRuleDoAdd( eq( index ) );
verifyNoMoreInteractions( indexCreationContext.schemaWriteOperations() );
verify( constraintCreationContext.schemaReadOperations() ).indexGetCommittedId( state, index, UNIQUE );
verify( constraintCreationContext.schemaReadOperations() ).indexGetCommittedId( state, index );
verifyNoMoreInteractions( constraintCreationContext.schemaReadOperations() );
verify( indexProxy ).awaitStoreScanCompleted();
}
Expand All @@ -117,7 +113,7 @@ public void shouldDropIndexIfPopulationFails() throws Exception
IndexingService indexingService = mock( IndexingService.class );
StubKernel kernel = new StubKernel();

when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index, UNIQUE ) )
when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index ) )
.thenReturn( 2468L );
IndexProxy indexProxy = mock( IndexProxy.class );
when( indexingService.getIndexProxy( 2468L ) ).thenReturn( indexProxy );
Expand Down Expand Up @@ -146,7 +142,7 @@ public void shouldDropIndexIfPopulationFails() throws Exception
NewIndexDescriptor newIndex = NewIndexDescriptorFactory.uniqueForLabel( 123, 456 );
verify( tx1 ).indexRuleDoAdd( newIndex );
verifyNoMoreInteractions( tx1 );
verify( constraintCreationContext.schemaReadOperations() ).indexGetCommittedId( state, index, UNIQUE );
verify( constraintCreationContext.schemaReadOperations() ).indexGetCommittedId( state, index );
verifyNoMoreInteractions( constraintCreationContext.schemaReadOperations() );
TransactionState tx2 = kernel.statements.get( 1 ).txState();
verify( tx2 ).indexDoDrop( newIndex );
Expand Down Expand Up @@ -185,7 +181,7 @@ public void shouldReleaseSchemaLockWhileAwaitingIndexPopulation() throws Excepti

KernelStatement state = mockedState();

when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index, UNIQUE ) )
when( constraintCreationContext.schemaReadOperations().indexGetCommittedId( state, index ) )
.thenReturn( 2468L );
IndexProxy indexProxy = mock( IndexProxy.class );
when( indexingService.getIndexProxy( anyLong() ) ).thenReturn( indexProxy );
Expand Down
Expand Up @@ -24,7 +24,6 @@
import org.junit.Rule;
import org.junit.Test;

import java.util.Iterator;
import java.util.concurrent.TimeUnit;

import org.neo4j.graphdb.GraphDatabaseService;
Expand All @@ -34,12 +33,9 @@
import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
import org.neo4j.graphdb.mockfs.UncloseableDelegatingFileSystemAbstraction;
import org.neo4j.graphdb.schema.IndexDefinition;
import org.neo4j.kernel.api.schema.NodePropertyDescriptor;
import org.neo4j.kernel.api.Statement;
import org.neo4j.kernel.api.schema.IndexDescriptor;
import org.neo4j.kernel.api.schema.IndexDescriptorFactory;
import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor;
import org.neo4j.kernel.api.schema_new.SchemaDescriptorFactory;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptorFactory;
import org.neo4j.kernel.impl.api.CountsAccessor;
import org.neo4j.kernel.impl.api.index.inmemory.InMemoryIndexProvider;
import org.neo4j.kernel.impl.api.index.inmemory.InMemoryIndexProviderFactory;
Expand All @@ -52,7 +48,6 @@
import org.neo4j.kernel.internal.GraphDatabaseAPI;
import org.neo4j.logging.AssertableLogProvider;
import org.neo4j.register.Register.DoubleLongRegister;
import org.neo4j.storageengine.api.schema.SchemaRule;
import org.neo4j.test.TestGraphDatabaseFactory;
import org.neo4j.test.rule.fs.EphemeralFileSystemRule;

Expand All @@ -64,8 +59,8 @@

public class IndexStatisticsIT
{
public static final Label ALIEN = label( "Alien" );
public static final String SPECIMEN = "specimen";
private static final Label ALIEN = label( "Alien" );
private static final String SPECIMEN = "specimen";

@Rule
public final EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule();
Expand Down Expand Up @@ -102,8 +97,9 @@ public void shouldRecoverIndexCountsBySamplingThemOnStartup()
awaitIndexOnline( indexAliensBySpecimen() );

// where ALIEN and SPECIMEN are both the first ids of their kind
IndexDescriptor index = IndexDescriptorFactory.of( labelId( ALIEN ), pkId( SPECIMEN ) );
long indexId = indexId( index );
NewIndexDescriptor index = NewIndexDescriptorFactory.forLabel( labelId( ALIEN ), pkId( SPECIMEN ) );
SchemaStorage storage = new SchemaStorage( neoStores().getSchemaStore() );
long indexId = storage.indexGetForSchema( index ).getId();

// for which we don't have index counts
resetIndexCounts( indexId );
Expand Down Expand Up @@ -195,14 +191,6 @@ private IndexDefinition indexAliensBySpecimen()
}
}

private long indexId( IndexDescriptor index )
{
SchemaStorage storage = new SchemaStorage( neoStores().getSchemaStore() );
LabelSchemaDescriptor descriptor =
SchemaDescriptorFactory.forLabel( index.getLabelId(), index.getPropertyKeyId() );
return storage.indexGetForSchema( descriptor ).getId();
}

private void resetIndexCounts( long indexId )
{
try ( CountsAccessor.IndexStatsUpdater updater = neoStores().getCounts().updateIndexCounts() )
Expand Down
Expand Up @@ -52,8 +52,6 @@
import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
import org.neo4j.kernel.api.schema.IndexDescriptor;
import org.neo4j.kernel.api.properties.Property;
import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor;
import org.neo4j.kernel.api.schema_new.SchemaDescriptorFactory;
import org.neo4j.kernel.api.schema_new.index.IndexBoundary;
import org.neo4j.kernel.api.schema_new.index.NewIndexDescriptor;
import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
Expand All @@ -65,7 +63,6 @@
import org.neo4j.kernel.monitoring.Monitors;
import org.neo4j.register.Register.DoubleLongRegister;
import org.neo4j.register.Registers;
import org.neo4j.storageengine.api.schema.SchemaRule;
import org.neo4j.test.rule.DatabaseRule;
import org.neo4j.test.rule.EmbeddedDatabaseRule;

Expand Down Expand Up @@ -181,7 +178,8 @@ public void shouldRemoveIndexStatisticsAfterIndexIsDeleted() throws KernelExcept
// given
createSomePersons();
NewIndexDescriptor index = awaitOnline( createIndex( "Person", "name" ) );
long indexId = indexId( index );
SchemaStorage storage = new SchemaStorage( neoStores().getSchemaStore() );
long indexId = storage.indexGetForSchema( index ).getId();

// when
dropIndex( index );
Expand Down Expand Up @@ -504,12 +502,6 @@ private NewIndexDescriptor createIndex( String labelName, String propertyKeyName
}
}

private long indexId( NewIndexDescriptor index )
{
SchemaStorage storage = new SchemaStorage( neoStores().getSchemaStore() );
return storage.indexGetForSchema( index.schema() ).getId();
}

private NeoStores neoStores()
{
return ( (GraphDatabaseAPI) db ).getDependencyResolver().resolveDependency( RecordStorageEngine.class )
Expand Down

0 comments on commit 79550ea

Please sign in to comment.