From 818d7420bb5f1b1354cd17624895b85e388c7c86 Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Wed, 12 Apr 2017 12:15:44 +0200 Subject: [PATCH] GBPTree can start as read only --- .../neo4j/index/internal/gbptree/GBPTree.java | 31 +++- .../gbptree/CrashGenerationCleanerTest.java | 32 +--- .../gbptree/FormatCompatibilityTest.java | 4 +- .../gbptree/GBPTreeConcurrencyIT.java | 2 +- .../index/internal/gbptree/GBPTreeIT.java | 2 +- .../internal/gbptree/GBPTreeRecoveryIT.java | 2 +- .../index/internal/gbptree/GBPTreeTest.java | 153 +++++++++++++++++- .../SimpleRecoveryCompleteMonitor.java | 36 +++++ .../index/labelscan/NativeLabelScanStore.java | 2 +- 9 files changed, 232 insertions(+), 32 deletions(-) create mode 100644 community/index/src/test/java/org/neo4j/index/internal/gbptree/SimpleRecoveryCompleteMonitor.java diff --git a/community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java b/community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java index 8293a1a7b612..f0dad81346dd 100644 --- a/community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java +++ b/community/index/src/main/java/org/neo4j/index/internal/gbptree/GBPTree.java @@ -288,6 +288,11 @@ default void recoveryCompleted( long numberOfPagesVisited, long numberOfCleanedC */ private final Monitor monitor; + /** + * Indicate that index was opened in a read only mode. + */ + private final boolean readOnly; + /** * Whether or not this tree has been closed. Accessed and changed solely in * {@link #close()} to be able to close tree multiple times gracefully. @@ -300,6 +305,7 @@ default void recoveryCompleted( long numberOfPagesVisited, long numberOfCleanedC * be written in index header. * If the index exists it will be opened and the {@link Layout} will be matched with the information * in the header. At the very least {@link Layout#identifier()} will be matched. + * If index is opened in read only mode, no writes are allowed. * * @param pageCache {@link PageCache} to use to map index file * @param indexFile {@link File} containing the actual index @@ -310,13 +316,15 @@ default void recoveryCompleted( long numberOfPagesVisited, long numberOfCleanedC * @param monitor {@link Monitor} for monitoring {@link GBPTree}. * @param headerReader reads header data, previously written using {@link #checkpoint(IOLimiter, Consumer)} * or {@link #close()} + * @param readOnly true if tree should be opened in read only mode. * @throws IOException on page cache error */ public GBPTree( PageCache pageCache, File indexFile, Layout layout, int tentativePageSize, - Monitor monitor, Header.Reader headerReader ) throws IOException + Monitor monitor, Header.Reader headerReader, boolean readOnly ) throws IOException { this.indexFile = indexFile; this.monitor = monitor; + this.readOnly = readOnly; this.generation = Generation.generation( GenerationSafePointer.MIN_GENERATION, GenerationSafePointer.MIN_GENERATION + 1 ); long rootId = IdSpace.MIN_TREE_NODE_ID; setRoot( rootId, Generation.unstableGeneration( generation ) ); @@ -408,6 +416,10 @@ private PagedFile openOrCreate( PageCache pageCache, File indexFile, { // First time monitor.noStoreFile(); + if ( readOnly ) + { + throw new UnsupportedOperationException( "Can't create new GBPTree in read only mode." ); + } pageSize = pageSizeForCreation == 0 ? pageCache.pageSize() : pageSizeForCreation; if ( pageSize > pageCache.pageSize() ) { @@ -690,6 +702,11 @@ public void checkpoint( IOLimiter ioLimiter ) throws IOException private void checkpoint( IOLimiter ioLimiter, Header.Writer headerWriter ) throws IOException { + if ( readOnly ) + { + throw new UnsupportedOperationException( "Can't checkpoint in read only mode." ); + } + if ( !changesSinceLastCheckpoint && headerWriter == CARRY_OVER_PREVIOUS_HEADER ) { // No changes has happened since last checkpoint was called, no need to do another checkpoint @@ -774,6 +791,10 @@ public void close() throws IOException */ public Writer writer() throws IOException { + if ( readOnly ) + { + throw new UnsupportedOperationException( "Can't open writer in read only mode." ); + } if ( !writerTaken.compareAndSet( false, true ) ) { throw new IllegalStateException( "Writer in " + this + " is already acquired by someone else. " + @@ -823,6 +844,10 @@ private void setRoot( long rootId, long rootGeneration ) */ public void prepareForRecovery() throws IOException { + if ( readOnly ) + { + throw new UnsupportedOperationException( "Can't prepare for recovery in read only mode." ); + } if ( changesSinceLastCheckpoint ) { throw new IllegalStateException( "It seems that this method has been called in the wrong state. " + @@ -845,6 +870,10 @@ public void prepareForRecovery() throws IOException */ public void finishRecovery() throws IOException { + if ( readOnly ) + { + throw new UnsupportedOperationException( "Can't initiate finish recovery in read only mode." ); + } // For the time being there's an issue where update that come in before a crash // may be applied in a different order than when they get recovered. // This may have structural changes be different during recovery than what they were diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/CrashGenerationCleanerTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/CrashGenerationCleanerTest.java index af23aea2cd9d..b08bf067d524 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/CrashGenerationCleanerTest.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/CrashGenerationCleanerTest.java @@ -83,22 +83,6 @@ public class CrashGenerationCleanerTest crashed( successor() ) ); - private class SimpleMonitor implements GBPTree.Monitor - { - boolean recoveryCompleted; - private long numberOfPagesVisited; - private long numberOfCleanedCrashPointers; - - @Override - public void recoveryCompleted( long numberOfPagesVisited, long numberOfCleanedCrashPointers, - long durationMillis ) - { - recoveryCompleted = true; - this.numberOfPagesVisited = numberOfPagesVisited; - this.numberOfCleanedCrashPointers = numberOfCleanedCrashPointers; - } - } - @Before public void setupPagedFile() throws IOException { @@ -122,7 +106,7 @@ public void shouldNotCrashOnEmptyFile() throws Exception initializeFile( pagedFile, pages ); // WHEN - SimpleMonitor monitor = new SimpleMonitor(); + SimpleRecoveryCompleteMonitor monitor = new SimpleRecoveryCompleteMonitor(); crashGenerationCleaner( pagedFile, 0, pages.length, monitor ).clean(); // THEN @@ -141,7 +125,7 @@ public void shouldNotReportErrorsOnCleanPages() throws Exception initializeFile( pagedFile, pages ); // WHEN - SimpleMonitor monitor = new SimpleMonitor(); + SimpleRecoveryCompleteMonitor monitor = new SimpleRecoveryCompleteMonitor(); crashGenerationCleaner( pagedFile, 0, pages.length, monitor ).clean(); // THEN @@ -174,7 +158,7 @@ public void shouldCleanOneCrashPerPage() throws Exception initializeFile( pagedFile, pages ); // WHEN - SimpleMonitor monitor = new SimpleMonitor(); + SimpleRecoveryCompleteMonitor monitor = new SimpleRecoveryCompleteMonitor(); crashGenerationCleaner( pagedFile, 0, pages.length, monitor ).clean(); // THEN @@ -202,7 +186,7 @@ public void shouldCleanMultipleCrashPerPage() throws Exception initializeFile( pagedFile, pages ); // WHEN - SimpleMonitor monitor = new SimpleMonitor(); + SimpleRecoveryCompleteMonitor monitor = new SimpleRecoveryCompleteMonitor(); crashGenerationCleaner( pagedFile, 0, pages.length, monitor ).clean(); // THEN @@ -227,7 +211,7 @@ public void shouldCleanLargeFile() throws Exception initializeFile( pagedFile, pages ); // WHEN - SimpleMonitor monitor = new SimpleMonitor(); + SimpleRecoveryCompleteMonitor monitor = new SimpleRecoveryCompleteMonitor(); crashGenerationCleaner( pagedFile, 0, numberOfPages, monitor ).clean(); // THEN @@ -236,7 +220,7 @@ public void shouldCleanLargeFile() throws Exception } private CrashGenerationCleaner crashGenerationCleaner( PagedFile pagedFile, int lowTreeNodeId, int highTreeNodeId, - SimpleMonitor monitor ) + SimpleRecoveryCompleteMonitor monitor ) { return new CrashGenerationCleaner( pagedFile, corruptableTreeNode, lowTreeNodeId, highTreeNodeId, stableGeneration, unstableGeneration, monitor ); @@ -255,7 +239,7 @@ private void initializeFile( PagedFile pagedFile, Page... pages ) throws IOExcep } /* Assertions */ - private void assertCleanedCrashPointers( SimpleMonitor monitor, + private void assertCleanedCrashPointers( SimpleRecoveryCompleteMonitor monitor, int expectedNumberOfCleanedCrashPointers ) { assertEquals( "Expected number of cleaned crash pointers to be " + @@ -263,7 +247,7 @@ private void assertCleanedCrashPointers( SimpleMonitor monitor, expectedNumberOfCleanedCrashPointers, monitor.numberOfCleanedCrashPointers ); } - private void assertPagesVisisted( SimpleMonitor monitor, int expectedNumberOfPagesVisited ) + private void assertPagesVisisted( SimpleRecoveryCompleteMonitor monitor, int expectedNumberOfPagesVisited ) { assertEquals( "Expected number of visited pages to be " + expectedNumberOfPagesVisited + " but was " + monitor.numberOfPagesVisited, diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/FormatCompatibilityTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/FormatCompatibilityTest.java index 6b0cfb58d7e3..20bc13088d98 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/FormatCompatibilityTest.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/FormatCompatibilityTest.java @@ -98,7 +98,7 @@ public void shouldDetectFormatChange() throws Throwable // THEN everything should work, otherwise there has likely been a format change PageCache pageCache = pageCacheRule.getPageCache( fsRule.get() ); try ( GBPTree tree = - new GBPTree<>( pageCache, storeFile, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER ) ) + new GBPTree<>( pageCache, storeFile, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER, false ) ) { try { @@ -174,7 +174,7 @@ private void createAndZipTree( File storeFile ) throws IOException { PageCache pageCache = pageCacheRule.getPageCache( fsRule.get() ); try ( GBPTree tree = - new GBPTree<>( pageCache, storeFile, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER ) ) + new GBPTree<>( pageCache, storeFile, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER, false ) ) { MutableLong insertKey = new MutableLong(); MutableLong insertValue = new MutableLong(); diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeConcurrencyIT.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeConcurrencyIT.java index ab8b1eb940ac..476c7212859b 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeConcurrencyIT.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeConcurrencyIT.java @@ -107,7 +107,7 @@ private GBPTree createIndex( GBPTree.Monitor monitor ) PageCache pageCache = pageCacheRule.getPageCache( fs.get(), config().withPageSize( pageSize ).withAccessChecks( true ) ); return index = new GBPTree<>( pageCache, directory.file( "index" ), - layout, 0/*use whatever page cache says*/, monitor, NO_HEADER ); + layout, 0/*use whatever page cache says*/, monitor, NO_HEADER, false ); } @After diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeIT.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeIT.java index 3bc3ae25fb06..e601b98aeb6b 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeIT.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeIT.java @@ -76,7 +76,7 @@ private GBPTree createIndex( int pageSize, GBPTree.Moni { pageCache = pageCacheRule.getPageCache( fs.get(), config().withPageSize( pageSize ).withAccessChecks( true ) ); return index = new GBPTree<>( pageCache, directory.file( "index" ), - layout, 0/*use whatever page cache says*/, monitor, NO_HEADER ); + layout, 0/*use whatever page cache says*/, monitor, NO_HEADER, false ); } @After diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java index 73db2dc460e9..73e41e3d4639 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeRecoveryIT.java @@ -388,7 +388,7 @@ private long[] modificationData( int min, int max ) private static GBPTree createIndex( PageCache pageCache, File file ) throws IOException { - return new GBPTree<>( pageCache, file, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER ); + return new GBPTree<>( pageCache, file, new SimpleLongLayout(), 0, NO_MONITOR, NO_HEADER, false ); } private PageCache createPageCache() diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java index 7a2a6c813448..317ae9cbe605 100644 --- a/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/GBPTreeTest.java @@ -54,13 +54,17 @@ import org.neo4j.io.pagecache.PageCache; import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PagedFile; +import org.neo4j.io.pagecache.tracing.DefaultPageCacheTracer; +import org.neo4j.io.pagecache.tracing.PageCacheTracer; import org.neo4j.test.Barrier; import org.neo4j.test.rule.PageCacheRule; import org.neo4j.test.rule.RandomRule; import org.neo4j.test.rule.TestDirectory; import org.neo4j.test.rule.fs.DefaultFileSystemRule; +import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -746,6 +750,8 @@ public void shouldSplitCorrectly() throws Exception } } + /* Checkpoint tests */ + @Test public void shouldCheckpointAfterInitialCreation() throws Exception { @@ -782,6 +788,143 @@ public void shouldNotCheckpointOnCloseIfNoChangesHappened() throws Exception assertEquals( 1, checkpointCounter.count() ); } + /* Read only mode tests */ + + @Test + public void openIndexInReadOnlyMustNotWriteAnything() throws Exception + { + // GIVEN + try ( GBPTree ignored = index().build() ) + { + // simply create + } + PageCacheTracer tracer = new DefaultPageCacheTracer(); + PageCacheRule.PageCacheConfig config = PageCacheRule.config().withTracer( tracer ); + + // WHEN + try ( PageCache pageCache = pageCacheRule.getPageCache( fs, config ); + GBPTree ignored = index().readOnly().with( pageCache ).build() ) + { + assertThat( tracer.bytesWritten(), is( 0L ) ); + } + } + + @Test + public void checkpointInReadOnlyModeMustThrow() throws Exception + { + // GIVEN + try ( GBPTree ignored = index().build() ) + { + // simply create + } + + // WHEN + try ( GBPTree index = index().readOnly().build() ) + { + index.checkpoint( IOLimiter.unlimited() ); + fail( "Expected checkpoint in read only mode to throw" ); + } + catch ( UnsupportedOperationException e ) + { + // THEN + // good + assertThat( e.getMessage(), + allOf( containsString( "read only" ), containsString( "checkpoint" ) ) ); + } + } + + @Test + public void openWriterInReadOnlyMustThrow() throws Exception + { + // GIVEN + try ( GBPTree ignored = index().build() ) + { + // simply create + } + + // WHEN + try ( GBPTree index = index().readOnly().build() ) + { + index.writer(); + fail( "Expected open writer in read only mode to throw" ); + } + catch ( UnsupportedOperationException e ) + { + // THEN + // good + assertThat( e.getMessage(), + allOf( containsString( "read only" ), containsString( "writer" ) ) ); + } + } + + @Test + public void createNewIndexInReadOnlyModeMustThrow() throws Exception + { + // GIVEN + // no existing index + + // WHEN + try ( GBPTree ignored = index().readOnly().build() ) + { + fail( "Expected creation of new index in read only mode to throw" ); + } + catch ( UnsupportedOperationException e ) + { + // THEN + // good + assertThat( e.getMessage(), + allOf( containsString( "read only" ), containsString( "create" ) ) ); + } + } + + @Test + public void prepareForRecoveryInReadOnlyMustThrow() throws Exception + { + // GIVEN + try ( GBPTree ignored = index().build() ) + { + // simply create + } + + // WHEN + try ( GBPTree index = index().readOnly().build() ) + { + index.prepareForRecovery(); + fail( "Expected prepare for recovery in read only mode to throw" ); + } + catch ( UnsupportedOperationException e ) + { + // THEN + // good + assertThat( e.getMessage(), + allOf( containsString( "read only" ), containsString( "prepare for recovery" ) ) ); + } + } + + @Test + public void finishRecoveryInReadOnlyMustThrow() throws Exception + { + // GIVEN + try ( GBPTree ignored = index().build() ) + { + // simply create + } + + // WHEN + try ( GBPTree index = index().readOnly().build() ) + { + index.finishRecovery(); + fail( "Expected finish recovery in read only mode to throw" ); + } + catch ( UnsupportedOperationException e ) + { + // THEN + // good + assertThat( e.getMessage(), + allOf( containsString( "read only" ), containsString( "finish recovery" ) ) ); + } + } + private void wait( Future future )throws InterruptedException, ExecutionException { try @@ -813,6 +956,7 @@ private class GBPTreeBuilder private Header.Reader headerReader = NO_HEADER; private Layout layout = GBPTreeTest.layout; private PageCache specificPageCache; + private boolean readOnly = false; private GBPTreeBuilder withPageCachePageSize( int pageSize ) { @@ -850,6 +994,12 @@ private GBPTreeBuilder with( PageCache pageCache ) return this; } + private GBPTreeBuilder readOnly() + { + this.readOnly = true; + return this; + } + private GBPTree build() throws IOException { PageCache pageCacheToUse; @@ -867,7 +1017,8 @@ private GBPTree build() throws IOException pageCacheToUse = specificPageCache; } - return new GBPTree<>( pageCacheToUse, indexFile, layout, tentativePageSize, monitor, headerReader ); + return new GBPTree<>( pageCacheToUse, indexFile, layout, tentativePageSize, monitor, headerReader, + readOnly ); } } diff --git a/community/index/src/test/java/org/neo4j/index/internal/gbptree/SimpleRecoveryCompleteMonitor.java b/community/index/src/test/java/org/neo4j/index/internal/gbptree/SimpleRecoveryCompleteMonitor.java new file mode 100644 index 000000000000..c598f3d56dcd --- /dev/null +++ b/community/index/src/test/java/org/neo4j/index/internal/gbptree/SimpleRecoveryCompleteMonitor.java @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.index.internal.gbptree; + +class SimpleRecoveryCompleteMonitor implements GBPTree.Monitor +{ + boolean recoveryCompleted; + long numberOfPagesVisited; + long numberOfCleanedCrashPointers; + + @Override + public void recoveryCompleted( long numberOfPagesVisited, long numberOfCleanedCrashPointers, + long durationMillis ) + { + recoveryCompleted = true; + this.numberOfPagesVisited = numberOfPagesVisited; + this.numberOfCleanedCrashPointers = numberOfCleanedCrashPointers; + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanStore.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanStore.java index 5050be63fcf7..a49551dd9ace 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanStore.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/labelscan/NativeLabelScanStore.java @@ -370,7 +370,7 @@ private boolean instantiateTree() throws IOException GBPTree.Monitor monitor = monitors.newMonitor( GBPTree.Monitor.class ); MutableBoolean isDirty = new MutableBoolean(); Header.Reader getDirty = (pageCursor, length) -> isDirty.setValue( pageCursor.getByte() == DIRTY ); - index = new GBPTree<>( pageCache, storeFile, new LabelScanLayout(), pageSize, monitor, getDirty ); + index = new GBPTree<>( pageCache, storeFile, new LabelScanLayout(), pageSize, monitor, getDirty, readOnly ); return isDirty.getValue(); }