From 4a29e2b6648ce578b04d8c80c867877415bd7fe3 Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 4 May 2016 15:46:46 +0200 Subject: [PATCH] Read unused dynamic records when reading chain Without short-lived locks reading of property records and corresponding dynamic records is not atomic. First, property record is read and only then corresponding dynamic records are read. This might result in reading inconsistent property values under concurrent load. This commit allows reading of unused dynamic records and their data when they are reachable from the chain. Co-authored-by: @MishaDemianenko --- .../api/store/StorePropertyPayloadCursor.java | 2 +- .../impl/store/AbstractDynamicStore.java | 67 +++--- .../kernel/impl/store/PropertyStore.java | 43 ++-- .../api/store/StorePropertyCursorTest.java | 195 +++++++++++++++++- .../store/StorePropertyPayloadCursorTest.java | 3 +- .../neo4j/kernel/impl/core/NodeProxyTest.java | 79 +++++++ .../impl/core/RelationshipProxyTest.java | 84 ++++++++ .../impl/store/AbstractDynamicStoreTest.java | 70 ++++++- 8 files changed, 463 insertions(+), 80 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursor.java index 981b6aca5b71b..ef699c69ac337 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursor.java @@ -278,7 +278,7 @@ private void readFromStore( AbstractDynamicStore store, AbstractDynamicStore.Dyn { buffer.clear(); long startBlockId = PropertyBlock.fetchLong( currentHeader() ); - try ( GenericCursor records = store.getRecordsCursor( startBlockId, true, cursor ) ) + try ( GenericCursor records = store.getRecordsCursor( startBlockId, cursor ) ) { while ( records.next() ) { diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java index 923b10b6f4900..0dd4ef7dc27da 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/AbstractDynamicStore.java @@ -67,16 +67,10 @@ public abstract class AbstractDynamicStore extends CommonAbstractStore implements RecordStore, DynamicBlockSize, DynamicRecordAllocator { - public static final byte[] NO_DATA = new byte[0]; + private static final byte[] NO_DATA = new byte[0]; // (in_use+next high)(1 byte)+nr_of_bytes(3 bytes)+next_block(int) public static final int BLOCK_HEADER_SIZE = 1 + 3 + 4; // = 8 - // Return signals for the readRecordHeader() method: - private static int hasDataSignal = 0; - private static int hasNoDataSignal = 1; - private static int notInUseSignal = 2; - private static int illegalSizeSignal = 3; - private final int blockSizeFromConfiguration; private int blockSize; @@ -393,12 +387,12 @@ private Collection getRecords( long startBlockId, boolean readBot while ( blockId != noNextBlock && cursor.next( pageIdForRecord( blockId ) ) ) { DynamicRecord record = new DynamicRecord( blockId ); - int headerReadResult; + HeaderReadResult headerReadResult; do { cursor.setOffset( offsetForId( blockId ) ); headerReadResult = readRecordHeader( cursor, record, false ); - if ( headerReadResult == hasDataSignal && readBothHeaderAndData ) + if ( headerReadResult == HeaderReadResult.DATA && readBothHeaderAndData ) { readRecordData( cursor, record ); } @@ -423,20 +417,17 @@ public DynamicRecordCursor newDynamicRecordCursor() return new DynamicRecordCursor(); } - public DynamicRecordCursor getRecordsCursor( final long startBlockId, - final boolean readBothHeaderAndData ) + DynamicRecordCursor getRecordsCursor( long startBlockId ) { - return getRecordsCursor( startBlockId, readBothHeaderAndData, newDynamicRecordCursor() ); + return getRecordsCursor( startBlockId, newDynamicRecordCursor() ); } - public DynamicRecordCursor getRecordsCursor( final long startBlockId, - final boolean readBothHeaderAndData, DynamicRecordCursor dynamicRecordCursor ) + public DynamicRecordCursor getRecordsCursor( long startBlockId, DynamicRecordCursor dynamicRecordCursor ) { try { - final PageCursor cursor = storeFile.io( 0, PF_SHARED_LOCK ); - - dynamicRecordCursor.init( startBlockId, cursor, readBothHeaderAndData ); + PageCursor cursor = storeFile.io( 0, PF_SHARED_LOCK ); + dynamicRecordCursor.init( startBlockId, cursor ); return dynamicRecordCursor; } catch ( IOException e ) @@ -445,17 +436,17 @@ public DynamicRecordCursor getRecordsCursor( final long startBlockId, } } - private void checkForInUse( int headerReadResult, DynamicRecord record ) + private void checkForInUse( HeaderReadResult headerReadResult, DynamicRecord record ) { - if ( headerReadResult == notInUseSignal ) + if ( headerReadResult == HeaderReadResult.NOT_IN_USE ) { - throw new InvalidRecordException( "DynamicRecord Not in use, blockId[" + record.getId() + "]" ); + throw new InvalidRecordException( "DynamicRecord not in use, blockId[" + record.getId() + "]" ); } } - private void checkForIllegalSize( int headerReadResult, DynamicRecord record ) + private void checkForIllegalSize( HeaderReadResult headerReadResult, DynamicRecord record ) { - if ( headerReadResult == illegalSizeSignal ) + if ( headerReadResult == HeaderReadResult.ILLEGAL_SIZE ) { int dataSize = getBlockSize() - AbstractDynamicStore.BLOCK_HEADER_SIZE; throw new InvalidRecordException( "Next block set[" + record.getNextBlock() @@ -467,7 +458,7 @@ private void checkForIllegalSize( int headerReadResult, DynamicRecord record ) /** * Reads data from the cursor into the given record, and returns one of the signals specified above. */ - private int readRecordHeader( PageCursor cursor, DynamicRecord record, boolean force ) + private HeaderReadResult readRecordHeader( PageCursor cursor, DynamicRecord record, boolean force ) { /* * First 4b @@ -484,7 +475,7 @@ private int readRecordHeader( PageCursor cursor, DynamicRecord record, boolean f boolean inUse = highNibbleInMaskedInteger == Record.IN_USE.intValue(); if ( !inUse && !force ) { - return notInUseSignal; + return HeaderReadResult.NOT_IN_USE; } int dataSize = getBlockSize() - AbstractDynamicStore.BLOCK_HEADER_SIZE; @@ -508,10 +499,10 @@ private int readRecordHeader( PageCursor cursor, DynamicRecord record, boolean f hasDataToRead = false; if ( !force ) { - return illegalSizeSignal; + return HeaderReadResult.ILLEGAL_SIZE; } } - return hasDataToRead ? hasDataSignal : hasNoDataSignal; + return hasDataToRead ? HeaderReadResult.DATA : HeaderReadResult.NO_DATA; } private void readRecordData( PageCursor cursor, DynamicRecord record ) @@ -566,7 +557,7 @@ public DynamicRecord getRecord( long id ) long pageId = pageIdForRecord( id ); try ( PageCursor cursor = storeFile.io( pageId, PF_SHARED_LOCK ) ) { - int headerReadResult = notInUseSignal; + HeaderReadResult headerReadResult = HeaderReadResult.NOT_IN_USE; if ( cursor.next() ) { int offset = offsetForId( record.getId() ); @@ -574,7 +565,7 @@ public DynamicRecord getRecord( long id ) { cursor.setOffset( offset ); headerReadResult = readRecordHeader( cursor, record, false ); - if ( headerReadResult == hasDataSignal ) + if ( headerReadResult == HeaderReadResult.DATA ) { readRecordData( cursor, record ); } @@ -602,12 +593,12 @@ public DynamicRecord forceGetRecord( long id ) if ( cursor.next() ) { int offset = offsetForId( record.getId() ); - int headerReadResult; + HeaderReadResult headerReadResult; do { cursor.setOffset( offset ); headerReadResult = readRecordHeader( cursor, record, true ); - if ( headerReadResult == hasDataSignal ) + if ( headerReadResult == HeaderReadResult.DATA ) { readRecordData( cursor, record ); } @@ -656,16 +647,14 @@ public String toString() public class DynamicRecordCursor extends GenericCursor { private PageCursor cursor; - private boolean readBothHeaderAndData; long blockId; int noNextBlock; private final DynamicRecord record = new DynamicRecord( blockId ); - public void init( long startBlockId, PageCursor cursor, boolean readBothHeaderAndData ) + public void init( long startBlockId, PageCursor cursor ) { this.cursor = cursor; - this.readBothHeaderAndData = readBothHeaderAndData; blockId = startBlockId; noNextBlock = Record.NO_NEXT_BLOCK.intValue(); } @@ -679,19 +668,18 @@ public boolean next() { record.setId( blockId ); - int headerReadResult; + HeaderReadResult headerReadResult; do { cursor.setOffset( offsetForId( blockId ) ); - headerReadResult = readRecordHeader( cursor, record, false ); - if ( headerReadResult == hasDataSignal && readBothHeaderAndData ) + headerReadResult = readRecordHeader( cursor, record, true ); + if ( headerReadResult == HeaderReadResult.DATA ) { readRecordData( cursor, record ); } } while ( cursor.shouldRetry() ); - checkForInUse( headerReadResult, record ); checkForIllegalSize( headerReadResult, record ); current = record; blockId = record.getNextBlock(); @@ -715,4 +703,9 @@ public void close() cursor = null; } } + + private enum HeaderReadResult + { + DATA, NO_DATA, NOT_IN_USE, ILLEGAL_SIZE + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java index 8e15d3d52ea33..da61e6b796f79 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java @@ -265,38 +265,33 @@ public void ensureHeavy( PropertyBlock block ) { if ( block.getType() == PropertyType.STRING ) { - if ( block.isLight() ) - { - try ( GenericCursor stringRecords = stringPropertyStore.getRecordsCursor( - block.getSingleValueLong(), false ) ) - { - while ( stringRecords.next() ) - { - stringRecords.get().setType( PropertyType.STRING.intValue() ); - block.addValueRecord( stringRecords.get().clone() ); - } - } - } - for ( DynamicRecord stringRecord : block.getValueRecords() ) - { - stringPropertyStore.ensureHeavy( stringRecord ); - } + loadPropertyBlock( block, stringPropertyStore ); } else if ( block.getType() == PropertyType.ARRAY ) { - if ( block.isLight() ) + loadPropertyBlock( block, arrayPropertyStore ); + } + } + + private static void loadPropertyBlock( PropertyBlock block, AbstractDynamicStore dynamicStore ) + { + if ( block.isLight() ) + { + long startBlockId = block.getSingleValueLong(); + try ( GenericCursor cursor = dynamicStore.getRecordsCursor( startBlockId ) ) { - Collection arrayRecords = arrayPropertyStore.getLightRecords( - block.getSingleValueLong() ); - for ( DynamicRecord arrayRecord : arrayRecords ) + while ( cursor.next() ) { - arrayRecord.setType( PropertyType.ARRAY.intValue() ); - block.addValueRecord( arrayRecord ); + cursor.get().setType( block.getType().intValue() ); + block.addValueRecord( cursor.get().clone() ); } } - for ( DynamicRecord arrayRecord : block.getValueRecords() ) + } + else + { + for ( DynamicRecord dynamicRecord : block.getValueRecords() ) { - arrayPropertyStore.ensureHeavy( arrayRecord ); + dynamicStore.ensureHeavy( dynamicRecord ); } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java index 7e8c7cb83e7c9..bbafb9d98dd9f 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyCursorTest.java @@ -42,6 +42,7 @@ import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; import org.neo4j.kernel.api.cursor.PropertyItem; +import org.neo4j.kernel.impl.store.AbstractRecordStore; import org.neo4j.kernel.impl.store.DynamicArrayStore; import org.neo4j.kernel.impl.store.DynamicRecordAllocator; import org.neo4j.kernel.impl.store.DynamicStringStore; @@ -49,6 +50,8 @@ import org.neo4j.kernel.impl.store.PropertyStore; import org.neo4j.kernel.impl.store.PropertyType; import org.neo4j.kernel.impl.store.StoreFactory; +import org.neo4j.kernel.impl.store.record.Abstract64BitRecord; +import org.neo4j.kernel.impl.store.record.DynamicRecord; import org.neo4j.kernel.impl.store.record.PropertyBlock; import org.neo4j.kernel.impl.store.record.PropertyRecord; import org.neo4j.logging.LogProvider; @@ -57,6 +60,8 @@ import org.neo4j.test.PageCacheRule; import static java.util.Arrays.asList; +import static org.apache.commons.lang3.RandomStringUtils.randomAscii; +import static org.apache.commons.lang3.RandomUtils.nextBytes; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -592,6 +597,156 @@ public void skipAllRecordsWhenWholeChainNotInUse() assertEquals( Collections.emptyList(), valuesFromCursor ); } } + + @Test + public void readPropertyChainWithLongStringDynamicRecordsNotInUse() + { + int chainStartId = 1; + int keyId = 42; + Object[] values = {randomAscii( 255 ), randomAscii( 255 ), randomAscii( 255 )}; + List propertyChain = createPropertyChain( propertyStore, chainStartId, keyId, values ); + + markDynamicRecordNotInUse( 2, keyId, propertyChain.get( 1 ), propertyStore ); + + try ( StorePropertyCursor cursor = newStorePropertyCursor( propertyStore ) ) + { + cursor.init( chainStartId, NO_LOCK ); + + List valuesFromCursor = asPropertyValuesList( cursor ); + assertEquals( asList( values ), valuesFromCursor ); + } + } + + @Test + public void readPropertyValueWhenFirstLongStringDynamicRecordIsNotInUse() + { + int recordId = 1; + int keyId = 1; + String value = randomAscii( 255 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + markDynamicRecordNotInUse( 0, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + @Test + public void readPropertyValueWhenSomeLongStringDynamicRecordsAreNotInUse() + { + int recordId = 1; + int keyId = 1; + String value = randomAscii( 1000 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + + markDynamicRecordNotInUse( 1, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 3, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 5, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 7, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + @Test + public void readPropertyValueWhenAllLongStringDynamicRecordsAreNotInUse() + { + int recordId = 1; + int keyId = 1; + String value = randomAscii( 255 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + + markDynamicRecordNotInUse( 0, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 1, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 2, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + @Test + public void readPropertyValueWhenAllLongArrayDynamicRecordsAreNotInUse() + { + int recordId = 1; + int keyId = 1; + byte[] value = nextBytes( 320 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + + markDynamicRecordNotInUse( 0, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 1, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 2, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + @Test + public void readPropertyChainWithLongArrayDynamicRecordsNotInUse() + { + int chainStartId = 1; + int keyId = 42; + Object[] values = {nextBytes( 1024 ), nextBytes( 1024 ), nextBytes( 1024 )}; + List propertyChain = createPropertyChain( propertyStore, chainStartId, keyId, values ); + + markDynamicRecordNotInUse( 2, keyId, propertyChain.get( 1 ), propertyStore ); + + try ( StorePropertyCursor cursor = newStorePropertyCursor( propertyStore ) ) + { + cursor.init( chainStartId, NO_LOCK ); + + List valuesFromCursor = asPropertyValuesList( cursor ); + for ( int i = 0; i < valuesFromCursor.size(); i++ ) + { + Object value = valuesFromCursor.get( i ); + assertArrayEquals( (byte[]) values[i], (byte[]) value ); + } + } + } + + @Test + public void readPropertyValueWhenFirstLongArrayDynamicRecordIsNotInUse() + { + int recordId = 1; + int keyId = 1; + byte[] value = nextBytes( 1024 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + markDynamicRecordNotInUse( 0, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + @Test + public void readPropertyValueWhenSomeLongArrayDynamicRecordsAreNotInUse() + { + int recordId = 1; + int keyId = 1; + byte[] value = nextBytes( 1024 ); + PropertyRecord record = createSinglePropertyValue( propertyStore, recordId, keyId, value ); + + markDynamicRecordNotInUse( 1, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 3, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 5, keyId, record, propertyStore ); + markDynamicRecordNotInUse( 7, keyId, record, propertyStore ); + + verifyPropertyValue( value, recordId ); + } + + private void verifyPropertyValue( String expectedValue, int recordId ) + { + try ( StorePropertyCursor cursor = newStorePropertyCursor( propertyStore ) ) + { + cursor.init( recordId, NO_LOCK ); + assertTrue( cursor.next() ); + assertEquals( expectedValue, cursor.value() ); + assertFalse( cursor.next() ); + } + } + + private void verifyPropertyValue( byte[] expectedValue, int recordId ) + { + try ( StorePropertyCursor cursor = newStorePropertyCursor( propertyStore ) ) + { + cursor.init( recordId, NO_LOCK ); + assertTrue( cursor.next() ); + assertArrayEquals( expectedValue, (byte[]) cursor.value() ); + assertFalse( cursor.next() ); + } + } } private static void assertEqualValues( Object expectedValue, PropertyItem item ) @@ -620,24 +775,30 @@ private static StorePropertyCursor newStorePropertyCursor( PropertyStore propert return new StorePropertyCursor( propertyStore, cache ); } - private static void createPropertyChain( PropertyStore store, int firstRecordId, int keyId, Object... values ) + private static List createPropertyChain( PropertyStore store, int firstRecordId, int keyId, + Object... values ) { + List records = new ArrayList<>(); + int nextRecordId = firstRecordId; - PropertyRecord previousRecord = null; for ( Object value : values ) { PropertyRecord record = createSinglePropertyValue( store, nextRecordId, keyId, value ); - if ( previousRecord != null ) + if ( !records.isEmpty() ) { + PropertyRecord previousRecord = records.get( records.size() - 1 ); + record.setPrevProp( previousRecord.getId() ); store.updateRecord( record ); previousRecord.setNextProp( record.getId() ); store.updateRecord( previousRecord ); } - previousRecord = record; + records.add( record ); nextRecordId++; } + + return records; } private static void markPropertyRecordsNoInUse( PropertyStore store, int... recordIds ) @@ -672,8 +833,7 @@ private static PropertyRecord createSinglePropertyValue( PropertyStore store, in PropertyRecord record = new PropertyRecord( recordId ); record.addPropertyBlock( block ); record.setInUse( true ); - store.updateRecord( record ); - store.setHighestPossibleIdInUse( recordId ); + updateRecord( store, record ); return record; } @@ -702,13 +862,22 @@ private static void createTwoPropertyValues( PropertyStore store, int recordId, nextRecord.addPropertyBlock( block2 ); nextRecord.setPrevProp( record.getId() ); nextRecord.setInUse( true ); - store.updateRecord( nextRecord ); - store.setHighestPossibleIdInUse( nextRecord.getId() ); + updateRecord( store, nextRecord ); } record.setInUse( true ); - store.updateRecord( record ); - store.setHighestPossibleIdInUse( record.getId() ); + updateRecord( store, record ); + } + + private static void markDynamicRecordNotInUse( int dynamicRecordIndex, int keyId, PropertyRecord record, + PropertyStore store ) + { + PropertyBlock propertyBlock = record.getPropertyBlock( keyId ); + store.ensureHeavy( propertyBlock ); + List valueRecords = propertyBlock.getValueRecords(); + DynamicRecord dynamicRecord = valueRecords.get( dynamicRecordIndex ); + dynamicRecord.setInUse( false ); + updateRecord( store, record ); } private static List asPropertyValuesList( StorePropertyCursor cursor ) @@ -720,4 +889,10 @@ private static List asPropertyValuesList( StorePropertyCursor cursor ) } return values; } + + private static void updateRecord( AbstractRecordStore store, T record ) + { + store.forceUpdateRecord( record ); + store.setHighestPossibleIdInUse( record.getLongId() ); + } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursorTest.java index 8204d257eeb3e..bf450319df090 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursorTest.java @@ -49,7 +49,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -489,7 +488,7 @@ private static S newDynamicStoreMock( Class S store = mock( clazz ); when( store.newDynamicRecordCursor() ).thenReturn( mock( AbstractDynamicStore.DynamicRecordCursor.class ) ); - when( store.getRecordsCursor( anyLong(), anyBoolean(), any( AbstractDynamicStore.DynamicRecordCursor.class ) ) ) + when( store.getRecordsCursor( anyLong(), any( AbstractDynamicStore.DynamicRecordCursor.class ) ) ) .thenReturn( recordCursor ); return store; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeProxyTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeProxyTest.java index 76931391119c9..97b9c402ea7f4 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeProxyTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/NodeProxyTest.java @@ -19,6 +19,8 @@ */ package org.neo4j.kernel.impl.core; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.commons.lang3.RandomUtils; import org.junit.Test; import java.util.UUID; @@ -27,15 +29,20 @@ import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicBoolean; +import org.neo4j.graphdb.DynamicLabel; +import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.NotFoundException; import org.neo4j.graphdb.PropertyContainer; import org.neo4j.graphdb.Transaction; +import org.neo4j.helpers.collection.IteratorUtil; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.neo4j.helpers.NamedThreadFactory.named; @@ -76,6 +83,78 @@ public void shouldThrowHumaneExceptionsWhenPropertyDoesNotExistOnNode() throws E } } + @Test + public void createDropNodeLongStringProperty() + { + Label markerLabel = DynamicLabel.label( "marker" ); + String testPropertyKey = "testProperty"; + String propertyValue = RandomStringUtils.randomAscii( 255 ); + + try ( Transaction tx = db.beginTx() ) + { + Node node = db.createNode( markerLabel ); + node.setProperty( testPropertyKey, propertyValue ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + assertEquals( propertyValue, node.getProperty( testPropertyKey ) ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + node.removeProperty( testPropertyKey ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + assertFalse( node.hasProperty( testPropertyKey ) ); + tx.success(); + } + } + + @Test + public void createDropNodeLongArrayProperty() + { + Label markerLabel = DynamicLabel.label( "marker" ); + String testPropertyKey = "testProperty"; + byte[] propertyValue = RandomUtils.nextBytes( 1024 ); + + try ( Transaction tx = db.beginTx() ) + { + Node node = db.createNode( markerLabel ); + node.setProperty( testPropertyKey, propertyValue ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + assertArrayEquals( propertyValue, (byte[]) node.getProperty( testPropertyKey ) ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + node.removeProperty( testPropertyKey ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Node node = IteratorUtil.single( db.findNodes( markerLabel ) ); + assertFalse( node.hasProperty( testPropertyKey ) ); + tx.success(); + } + } + @Test public void shouldThrowHumaneExceptionsWhenPropertyDoesNotExist() throws Exception { diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/RelationshipProxyTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/RelationshipProxyTest.java index 5fa5ae1c60841..b63dacb620186 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/core/RelationshipProxyTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/core/RelationshipProxyTest.java @@ -19,16 +19,24 @@ */ package org.neo4j.kernel.impl.core; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.commons.lang3.RandomUtils; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.neo4j.graphdb.DynamicLabel; +import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.PropertyContainer; +import org.neo4j.graphdb.Relationship; import org.neo4j.graphdb.RelationshipType; +import org.neo4j.graphdb.Transaction; import org.neo4j.kernel.impl.core.RelationshipProxy.RelationshipActions; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Mockito.mock; @@ -102,6 +110,82 @@ public RelationshipType answer( InvocationOnMock invocation ) throws Throwable } } + @Test + public void createDropRelationshipLongStringProperty() + { + Label markerLabel = DynamicLabel.label( "marker" ); + String testPropertyKey = "testProperty"; + String propertyValue = RandomStringUtils.randomAscii( 255 ); + + try ( Transaction tx = db.beginTx() ) + { + Node start = db.createNode( markerLabel ); + Node end = db.createNode( markerLabel ); + Relationship relationship = start.createRelationshipTo( end, withName( "type" ) ); + relationship.setProperty( testPropertyKey, propertyValue ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + assertEquals( propertyValue, relationship.getProperty( testPropertyKey ) ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + relationship.removeProperty( testPropertyKey ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + assertFalse( relationship.hasProperty( testPropertyKey ) ); + tx.success(); + } + } + + @Test + public void createDropRelationshipLongArrayProperty() + { + Label markerLabel = DynamicLabel.label( "marker" ); + String testPropertyKey = "testProperty"; + byte[] propertyValue = RandomUtils.nextBytes( 1024 ); + + try ( Transaction tx = db.beginTx() ) + { + Node start = db.createNode( markerLabel ); + Node end = db.createNode( markerLabel ); + Relationship relationship = start.createRelationshipTo( end, withName( "type" ) ); + relationship.setProperty( testPropertyKey, propertyValue ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + assertArrayEquals( propertyValue, (byte[]) relationship.getProperty( testPropertyKey ) ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + relationship.removeProperty( testPropertyKey ); + tx.success(); + } + + try ( Transaction tx = db.beginTx() ) + { + Relationship relationship = db.getRelationshipById( 0 ); + assertFalse( relationship.hasProperty( testPropertyKey ) ); + tx.success(); + } + } + private void verifyIds( RelationshipActions actions, long relationshipId, long nodeId1, int typeId, long nodeId2 ) { RelationshipProxy proxy = new RelationshipProxy( actions, relationshipId, nodeId1, typeId, nodeId2 ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/AbstractDynamicStoreTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/AbstractDynamicStoreTest.java index 1a6953ff52fad..6ebb7871ac5a0 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/AbstractDynamicStoreTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/AbstractDynamicStoreTest.java @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.store; +import org.apache.commons.lang3.RandomUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -38,6 +39,7 @@ import org.neo4j.test.EphemeralFileSystemRule; import org.neo4j.test.PageCacheRule; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -59,18 +61,13 @@ public void before() throws IOException { fs = fsr.get(); pageCache = pageCacheRule.getPageCache( fsr.get() ); - StoreChannel channel = fs.create( fileName ); - try + try ( StoreChannel channel = fs.create( fileName ) ) { ByteBuffer buffer = ByteBuffer.allocate( 4 ); buffer.putInt( BLOCK_SIZE ); buffer.flip(); channel.write( buffer ); } - finally - { - channel.close(); - } } @Test @@ -91,6 +88,67 @@ public void shouldRecognizeDesignatedInUseBit() throws Exception } } + @Test + public void dynamicRecordCursorReadsInUseRecords() + { + try ( AbstractDynamicStore store = newTestableDynamicStore() ) + { + DynamicRecord first = createDynamicRecord( 1, store ); + DynamicRecord second = createDynamicRecord( 2, store ); + DynamicRecord third = createDynamicRecord( 3, store ); + + first.setNextBlock( second.getId() ); + store.forceUpdateRecord( first ); + second.setNextBlock( third.getId() ); + store.forceUpdateRecord( second ); + + AbstractDynamicStore.DynamicRecordCursor recordsCursor = store.getRecordsCursor( 1 ); + assertTrue( recordsCursor.next() ); + assertEquals( first, recordsCursor.get() ); + assertTrue( recordsCursor.next() ); + assertEquals( second, recordsCursor.get() ); + assertTrue( recordsCursor.next() ); + assertEquals( third, recordsCursor.get() ); + assertFalse( recordsCursor.next() ); + } + } + + @Test + public void dynamicRecordCursorReadsNotInUseRecords() + { + try ( AbstractDynamicStore store = newTestableDynamicStore() ) + { + DynamicRecord first = createDynamicRecord( 1, store ); + DynamicRecord second = createDynamicRecord( 2, store ); + DynamicRecord third = createDynamicRecord( 3, store ); + + first.setNextBlock( second.getId() ); + store.forceUpdateRecord( first ); + second.setNextBlock( third.getId() ); + store.forceUpdateRecord( second ); + second.setInUse( false ); + store.forceUpdateRecord( second ); + + AbstractDynamicStore.DynamicRecordCursor recordsCursor = store.getRecordsCursor( 1 ); + assertTrue( recordsCursor.next() ); + assertEquals( first, recordsCursor.get() ); + assertTrue( recordsCursor.next() ); + assertEquals( second, recordsCursor.get() ); + assertTrue( recordsCursor.next() ); + assertEquals( third, recordsCursor.get() ); + assertFalse( recordsCursor.next() ); + } + } + + private static DynamicRecord createDynamicRecord( long id, AbstractDynamicStore store ) + { + DynamicRecord first = new DynamicRecord( id ); + first.setInUse( true ); + first.setData( RandomUtils.nextBytes( 10 ) ); + store.forceUpdateRecord( first ); + return first; + } + private void assertRecognizesByteAsInUse( AbstractDynamicStore store, byte inUseByte ) { assertTrue( store.isInUse( (byte) (inUseByte | 0x10) ) );