diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Bits.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Bits.java index 60c09575b999..fa4065059af9 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Bits.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/Bits.java @@ -177,6 +177,11 @@ public String toString() return builder.toString(); } + public static String numberToString( long value, int numberOfBytes ) + { + return numberToString( new StringBuilder(), value, numberOfBytes ).toString(); + } + public static StringBuilder numberToString( StringBuilder builder, long value, int numberOfBytes ) { builder.append( "[" ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/LowLimitRecordFormatTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/LowLimitRecordFormatTest.java index 1e5795eebced..5259fff750e3 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/LowLimitRecordFormatTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/LowLimitRecordFormatTest.java @@ -19,9 +19,6 @@ */ package org.neo4j.kernel.impl.store.format; -import org.neo4j.kernel.impl.store.format.LimitedRecordGenerators; -import org.neo4j.kernel.impl.store.format.RecordFormatTest; -import org.neo4j.kernel.impl.store.format.RecordGenerators; import org.neo4j.kernel.impl.store.format.lowlimit.LowLimit; public class LowLimitRecordFormatTest extends RecordFormatTest diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatTest.java index e5d352167874..b9dbbf7a2c47 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/format/RecordFormatTest.java @@ -64,7 +64,7 @@ public abstract class RecordFormatTest public static final RandomRule random = new RandomRule(); private final EphemeralFileSystemRule fsRule = new EphemeralFileSystemRule(); - private final PageCacheRule pageCacheRule = new PageCacheRule( false /*true here later*/ ); + private final PageCacheRule pageCacheRule = new PageCacheRule(); @Rule public final RuleChain ruleChain = RuleChain.outerRule( pageCacheRule ).around( fsRule ); @@ -160,46 +160,13 @@ private void verifyWriteAndRead( R written = generator.get( recordSize, format ); try { - // write - try ( PageCursor cursor = storeFile.io( 0, PagedFile.PF_SHARED_WRITE_LOCK ) ) - { - assertedNext( cursor ); - if ( written.inUse() ) - { - format.prepare( written, recordSize, idSequence ); - } + writeRecord( written, format, storeFile, recordSize, idSequence ); - int offset = Math.toIntExact( written.getId() * recordSize ); - cursor.setOffset( offset ); - format.write( written, cursor, recordSize, storeFile ); - } long recordsUsedForWriting = storeFile.nextCalls(); long unusedBytes = storeFile.unusedBytes(); storeFile.resetMeasurements(); - // read - try ( PageCursor cursor = storeFile.io( 0, PagedFile.PF_SHARED_READ_LOCK ) ) - { - assertedNext( cursor ); - int offset = Math.toIntExact( written.getId() * recordSize ); - cursor.setOffset( offset ); - @SuppressWarnings( "unchecked" ) - R read = (R) written.clone(); // just to get a new instance - format.read( read, cursor, NORMAL, recordSize, storeFile ); - - // THEN - if ( written.inUse() ) - { - assertEquals( written.inUse(), read.inUse() ); - assertEquals( written.getId(), read.getId() ); - assertEquals( written.getSecondaryUnitId(), read.getSecondaryUnitId() ); - key.assertRecordsEquals( written, read ); - } - else - { - assertEquals( written.inUse(), read.inUse() ); - } - } + readAndVerifyRecord( written, format, key, storeFile, recordSize ); if ( written.inUse() ) { @@ -248,6 +215,59 @@ private void verifyWriteAndRead( } } + private void readAndVerifyRecord( R written, RecordFormat format, + RecordKey key, PagedFile storeFile, int recordSize ) throws IOException + { + try ( PageCursor cursor = storeFile.io( 0, PagedFile.PF_SHARED_READ_LOCK ) ) + { + assertedNext( cursor ); + R read = format.newRecord(); + read.setId( written.getId() ); + + /** + Retry loop is needed here because format does not handle retries on the primary cursor. + Same retry is done on the store level in {@link org.neo4j.kernel.impl.store.CommonAbstractStore} + */ + int offset = Math.toIntExact( written.getId() * recordSize ); + do + { + cursor.setOffset( offset ); + format.read( read, cursor, NORMAL, recordSize, storeFile ); + } + while ( cursor.shouldRetry() ); + + // THEN + if ( written.inUse() ) + { + assertEquals( written.inUse(), read.inUse() ); + assertEquals( written.getId(), read.getId() ); + assertEquals( written.getSecondaryUnitId(), read.getSecondaryUnitId() ); + key.assertRecordsEquals( written, read ); + } + else + { + assertEquals( written.inUse(), read.inUse() ); + } + } + } + + private void writeRecord( R record, RecordFormat format, PagedFile storeFile, + int recordSize, BatchingIdSequence idSequence ) throws IOException + { + try ( PageCursor cursor = storeFile.io( 0, PagedFile.PF_SHARED_WRITE_LOCK ) ) + { + assertedNext( cursor ); + if ( record.inUse() ) + { + format.prepare( record, recordSize, idSequence ); + } + + int offset = Math.toIntExact( record.getId() * recordSize ); + cursor.setOffset( offset ); + format.write( record, cursor, recordSize, storeFile ); + } + } + private double percent( long part, long total ) { return 100.0D * part / total; diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/SecondaryPageCursorReadDataAdapter.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/SecondaryPageCursorReadDataAdapter.java index 4a2384547824..74d3642709cd 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/SecondaryPageCursorReadDataAdapter.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/store/format/highlimit/SecondaryPageCursorReadDataAdapter.java @@ -23,8 +23,8 @@ import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; -import org.neo4j.kernel.impl.store.format.highlimit.BaseHighLimitRecordFormat; import org.neo4j.kernel.impl.store.format.highlimit.Reference.DataAdapter; +import org.neo4j.kernel.impl.util.Bits; /** * {@link DataAdapter} able to acquire a secondary {@link PageCursor} on potentially a different page @@ -38,6 +38,7 @@ class SecondaryPageCursorReadDataAdapter implements DataAdapter, Sec private final PageCursor secondaryCursor; private final int secondaryOffset; private boolean switched; + private byte secondaryHeaderByte; SecondaryPageCursorReadDataAdapter( PageCursor cursor, PagedFile storeFile, long secondaryPageId, int secondaryOffset, int primaryEndOffset, int pfFlags ) throws IOException @@ -58,12 +59,9 @@ public byte get( PageCursor primaryCursor /*same as the one we have*/ ) // We've come to the end of the primary cursor, use the secondary cursor instead if ( !switched ) { - // Just read out the header, get it out of the way and verify that this secondary record - // is in fact a secondary record. - // TODO can we do this in BaseHighLimitRecordFormat (the place where this adapter is created) instead? - byte secondaryHeaderByte = secondaryCursor.getByte(); - assert (secondaryHeaderByte & BaseHighLimitRecordFormat.HEADER_BIT_RECORD_UNIT) != 0; - assert (secondaryHeaderByte & BaseHighLimitRecordFormat.HEADER_BIT_FIRST_RECORD_UNIT) == 0; + // Just read out the header, get it out of the way. Verification of the header byte happens in #close() + // because it is the only place where we surely read the consistent value + secondaryHeaderByte = secondaryCursor.getByte(); switched = true; } return secondaryCursor.getByte(); @@ -99,5 +97,13 @@ public boolean shouldRetry() throws IOException public void close() { secondaryCursor.close(); + + assert (secondaryHeaderByte & BaseHighLimitRecordFormat.HEADER_BIT_RECORD_UNIT) != 0 : illegalHeader(); + assert (secondaryHeaderByte & BaseHighLimitRecordFormat.HEADER_BIT_FIRST_RECORD_UNIT) == 0 : illegalHeader(); + } + + private String illegalHeader() + { + return "Read illegal secondary record unit header: " + Bits.numberToString( secondaryHeaderByte, Byte.BYTES ); } } diff --git a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitRecordFormatTest.java b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitRecordFormatTest.java index 0019dd6348ad..be4511644489 100644 --- a/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitRecordFormatTest.java +++ b/enterprise/kernel/src/test/java/org/neo4j/kernel/impl/store/format/highlimit/HighLimitRecordFormatTest.java @@ -22,14 +22,13 @@ import org.neo4j.kernel.impl.store.format.LimitedRecordGenerators; import org.neo4j.kernel.impl.store.format.RecordFormatTest; import org.neo4j.kernel.impl.store.format.RecordGenerators; -import org.neo4j.kernel.impl.store.format.highlimit.HighLimit; public class HighLimitRecordFormatTest extends RecordFormatTest { - protected static final RecordGenerators _50_BIT_LIMITS = new LimitedRecordGenerators( random, 50, 50, 50, 16, NULL ); + private static final RecordGenerators HIGH_LIMITS = new LimitedRecordGenerators( random, 50, 50, 50, 16, NULL ); public HighLimitRecordFormatTest() { - super( HighLimit.RECORD_FORMATS, _50_BIT_LIMITS ); + super( HighLimit.RECORD_FORMATS, HIGH_LIMITS ); } }