From 83de6971682094370fcbd8c9361a2377e2769adf Mon Sep 17 00:00:00 2001 From: MishaDemianenko Date: Tue, 21 Mar 2017 17:52:46 +0100 Subject: [PATCH] Cleanup and enhance tests --- .../legacy/MultipleBackupDeletionPolicy.java | 46 ------ .../legacy/WritableIndexReferenceFactory.java | 4 +- .../api/impl/index/IndexWriterConfigs.java | 5 +- community/neo4j/pom.xml | 1 - .../org/neo4j/index/backup/IndexBackupIT.java | 132 ++++++++++++++++-- 5 files changed, 125 insertions(+), 63 deletions(-) delete mode 100644 community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/MultipleBackupDeletionPolicy.java diff --git a/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/MultipleBackupDeletionPolicy.java b/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/MultipleBackupDeletionPolicy.java deleted file mode 100644 index 1d38f08263ef..000000000000 --- a/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/MultipleBackupDeletionPolicy.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * 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.impl.lucene.legacy; - -import org.apache.lucene.index.IndexCommit; -import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy; -import org.apache.lucene.index.SnapshotDeletionPolicy; - -import java.io.IOException; - -public class MultipleBackupDeletionPolicy extends SnapshotDeletionPolicy -{ - public MultipleBackupDeletionPolicy() - { - super( new KeepOnlyLastCommitDeletionPolicy() ); - } - - @Override - public synchronized IndexCommit snapshot() throws IOException - { - return super.snapshot(); - } - - @Override - public synchronized void release( IndexCommit commit ) throws IOException - { - super.release( commit ); - } -} diff --git a/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/WritableIndexReferenceFactory.java b/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/WritableIndexReferenceFactory.java index 20f124384418..c3214d20d004 100644 --- a/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/WritableIndexReferenceFactory.java +++ b/community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/legacy/WritableIndexReferenceFactory.java @@ -23,6 +23,8 @@ import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy; +import org.apache.lucene.index.SnapshotDeletionPolicy; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.store.Directory; @@ -89,7 +91,7 @@ private IndexWriter newIndexWriter( IndexIdentifier identifier ) Directory indexDirectory = getIndexDirectory( identifier ); IndexType type = getType( identifier ); IndexWriterConfig writerConfig = new IndexWriterConfig( type.analyzer ); - writerConfig.setIndexDeletionPolicy( new MultipleBackupDeletionPolicy() ); + writerConfig.setIndexDeletionPolicy( new SnapshotDeletionPolicy( new KeepOnlyLastCommitDeletionPolicy() ) ); Similarity similarity = type.getSimilarity(); if ( similarity != null ) { diff --git a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/IndexWriterConfigs.java b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/IndexWriterConfigs.java index 01ec6c93506b..c85ac35ebefb 100644 --- a/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/IndexWriterConfigs.java +++ b/community/lucene-index/src/main/java/org/neo4j/kernel/api/impl/index/IndexWriterConfigs.java @@ -23,10 +23,11 @@ import org.apache.lucene.codecs.blocktreeords.BlockTreeOrdsPostingsFormat; import org.apache.lucene.codecs.lucene54.Lucene54Codec; import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.KeepOnlyLastCommitDeletionPolicy; import org.apache.lucene.index.LogByteSizeMergePolicy; +import org.apache.lucene.index.SnapshotDeletionPolicy; import org.neo4j.index.impl.lucene.legacy.LuceneDataSource; -import org.neo4j.index.impl.lucene.legacy.MultipleBackupDeletionPolicy; import org.neo4j.unsafe.impl.internal.dragons.FeatureToggles; /** @@ -70,7 +71,7 @@ public static IndexWriterConfig standard() writerConfig.setMaxBufferedDocs( MAX_BUFFERED_DOCS ); writerConfig.setMaxBufferedDeleteTerms( MAX_BUFFERED_DELETE_TERMS ); - writerConfig.setIndexDeletionPolicy( new MultipleBackupDeletionPolicy() ); + writerConfig.setIndexDeletionPolicy( new SnapshotDeletionPolicy( new KeepOnlyLastCommitDeletionPolicy() ) ); writerConfig.setUseCompoundFile( true ); writerConfig.setRAMBufferSizeMB( STANDARD_RAM_BUFFER_SIZE_MB ); writerConfig.setCodec(new Lucene54Codec() diff --git a/community/neo4j/pom.xml b/community/neo4j/pom.xml index 90331913f326..7eafc83e5d2f 100644 --- a/community/neo4j/pom.xml +++ b/community/neo4j/pom.xml @@ -100,7 +100,6 @@ commons-io test - org.neo4j neo4j-common diff --git a/community/neo4j/src/test/java/org/neo4j/index/backup/IndexBackupIT.java b/community/neo4j/src/test/java/org/neo4j/index/backup/IndexBackupIT.java index 0ae729f84de0..f212cbc45b14 100644 --- a/community/neo4j/src/test/java/org/neo4j/index/backup/IndexBackupIT.java +++ b/community/neo4j/src/test/java/org/neo4j/index/backup/IndexBackupIT.java @@ -20,32 +20,57 @@ package org.neo4j.index.backup; import org.apache.commons.io.FilenameUtils; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import java.io.File; import java.io.IOException; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import java.util.stream.LongStream; import org.neo4j.graphdb.DependencyResolver; import org.neo4j.graphdb.Label; import org.neo4j.graphdb.Node; import org.neo4j.graphdb.ResourceIterator; import org.neo4j.graphdb.Transaction; +import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.kernel.impl.api.index.IndexingService; import org.neo4j.kernel.impl.transaction.log.checkpoint.CheckPointer; import org.neo4j.kernel.impl.transaction.log.checkpoint.SimpleTriggerInfo; import org.neo4j.test.rule.EmbeddedDatabaseRule; +import org.neo4j.test.rule.RandomRule; +import static java.lang.String.format; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasSize; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; public class IndexBackupIT { + private static final String PROPERTY_PREFIX = "property"; + private static final int NUMBER_OF_INDEXES = 10; + + @Rule + public RandomRule randomRule = new RandomRule(); @Rule - public EmbeddedDatabaseRule database = new EmbeddedDatabaseRule( getClass() ).startLazily(); + public EmbeddedDatabaseRule database = new EmbeddedDatabaseRule( getClass() ); + private CheckPointer checkPointer; + private IndexingService indexingService; + private FileSystemAbstraction fileSystem; + + @Before + public void setUp() + { + checkPointer = resolveDependency( CheckPointer.class ); + indexingService = resolveDependency( IndexingService.class ); + fileSystem = resolveDependency( FileSystemAbstraction.class ); + } @Test public void concurrentIndexSnapshotUseDifferentSnapshots() throws Exception @@ -53,31 +78,113 @@ public void concurrentIndexSnapshotUseDifferentSnapshots() throws Exception Label label = Label.label( "testLabel" ); prepareDatabase( label ); - CheckPointer checkPointer = resolveDependency( CheckPointer.class ); - IndexingService indexingService = resolveDependency( IndexingService.class ); - forceCheckpoint( checkPointer ); ResourceIterator firstCheckpointSnapshot = indexingService.snapshotStoreFiles(); generateData( label ); + removeOldNodes( LongStream.range( 1, 20 ) ); + updateOldNodes( LongStream.range( 30, 40 ) ); forceCheckpoint( checkPointer ); ResourceIterator secondCheckpointSnapshot = indexingService.snapshotStoreFiles(); + generateData( label ); + removeOldNodes( LongStream.range( 50, 60 ) ); + updateOldNodes( LongStream.range( 70, 80 ) ); + + forceCheckpoint( checkPointer ); + ResourceIterator thirdCheckpointSnapshot = indexingService.snapshotStoreFiles(); + + Set firstSnapshotFileNames = getFileNames( firstCheckpointSnapshot ); + Set secondSnapshotFileNames = getFileNames( secondCheckpointSnapshot ); + Set thirdSnapshotFileNames = getFileNames( thirdCheckpointSnapshot ); + + compareSnapshotFiles( firstSnapshotFileNames, secondSnapshotFileNames, fileSystem ); + compareSnapshotFiles( secondSnapshotFileNames, thirdSnapshotFileNames, fileSystem); + compareSnapshotFiles( thirdSnapshotFileNames, firstSnapshotFileNames, fileSystem); + + firstCheckpointSnapshot.close(); + secondCheckpointSnapshot.close(); + thirdCheckpointSnapshot.close(); + + } + + @Test + public void snapshotFilesDeletedWhenSnapshotReleased() throws IOException + { + Label label = Label.label( "testLabel" ); + prepareDatabase( label ); + + ResourceIterator firstCheckpointSnapshot = indexingService.snapshotStoreFiles(); + generateData( label ); + ResourceIterator secondCheckpointSnapshot = indexingService.snapshotStoreFiles(); + generateData( label ); + ResourceIterator thirdCheckpointSnapshot = indexingService.snapshotStoreFiles(); + Set firstSnapshotFileNames = getFileNames( firstCheckpointSnapshot ); Set secondSnapshotFileNames = getFileNames( secondCheckpointSnapshot ); + Set thirdSnapshotFileNames = getFileNames( thirdCheckpointSnapshot ); + + generateData( label ); + forceCheckpoint( checkPointer ); + + assertTrue( firstSnapshotFileNames.stream().map( File::new ).allMatch( fileSystem::fileExists ) ); + assertTrue( secondSnapshotFileNames.stream().map( File::new ).allMatch( fileSystem::fileExists ) ); + assertTrue( thirdSnapshotFileNames.stream().map( File::new ).allMatch( fileSystem::fileExists ) ); + + firstCheckpointSnapshot.close(); + secondCheckpointSnapshot.close(); + thirdCheckpointSnapshot.close(); - for ( String nameInFirstSnapshot : firstSnapshotFileNames ) + generateData( label ); + forceCheckpoint( checkPointer ); + + assertFalse( firstSnapshotFileNames.stream().map( File::new ).anyMatch( fileSystem::fileExists ) ); + assertFalse( secondSnapshotFileNames.stream().map( File::new ).anyMatch( fileSystem::fileExists ) ); + assertFalse( thirdSnapshotFileNames.stream().map( File::new ).anyMatch( fileSystem::fileExists ) ); + } + + private void compareSnapshotFiles( Set firstSnapshotFileNames, Set secondSnapshotFileNames, + FileSystemAbstraction fileSystem ) + { + assertThat( + format( "Should have at least %d modified index files. Snapshot files are: %s", NUMBER_OF_INDEXES + 1, + firstSnapshotFileNames ), firstSnapshotFileNames, + hasSize( greaterThanOrEqualTo( NUMBER_OF_INDEXES + 1 ) ) ); + for ( String fileName : firstSnapshotFileNames ) { - assertFalse( "Second snapshot fileset should not have files from first snapshot set." + + assertFalse( "Snapshot fileset should not have files from another snapshot set." + describeFileSets( firstSnapshotFileNames, secondSnapshotFileNames ), - secondSnapshotFileNames.contains( nameInFirstSnapshot ) ); - String path = FilenameUtils.getFullPath( nameInFirstSnapshot ); + secondSnapshotFileNames.contains( fileName ) ); + String path = FilenameUtils.getFullPath( fileName ); assertTrue( "Snapshot should contain files for index in path: " + path + "." + describeFileSets( firstSnapshotFileNames, secondSnapshotFileNames ), secondSnapshotFileNames.stream().anyMatch( name -> name.startsWith( path ) ) ); + assertTrue( format( "Snapshot file '%s' should exist.", fileName ), + fileSystem.fileExists( new File( fileName ) ) ); + } + } + + private void removeOldNodes( LongStream idRange ) + { + try ( Transaction transaction = database.beginTx() ) + { + idRange.mapToObj( id -> database.getNodeById( id ) ).forEach( Node::delete ); + transaction.success(); + } + } + + private void updateOldNodes( LongStream idRange ) + { + try ( Transaction transaction = database.beginTx() ) + { + List nodes = idRange.mapToObj( id -> database.getNodeById( id ) ).collect( Collectors.toList() ); + for ( int i = 0; i < NUMBER_OF_INDEXES; i++ ) + { + String propertyName = PROPERTY_PREFIX + i; + nodes.forEach( node -> node.setProperty( propertyName, randomRule.nextLong() ) ); + } + transaction.success(); } - firstCheckpointSnapshot.close(); - secondCheckpointSnapshot.close(); } private String describeFileSets(Set firstFileSet, Set secondFileSet) @@ -104,7 +211,7 @@ private void prepareDatabase( Label label ) { for ( int i = 0; i < 10; i++ ) { - database.schema().indexFor( label ).on( "property" + i ).create(); + database.schema().indexFor( label ).on( PROPERTY_PREFIX + i ).create(); } transaction.success(); } @@ -117,7 +224,7 @@ private void prepareDatabase( Label label ) private void generateData( Label label ) { - for ( int i = 0; i < 10; i++ ) + for ( int i = 0; i < 100; i++ ) { testNodeCreationTransaction( label, i ); } @@ -129,7 +236,6 @@ private void testNodeCreationTransaction( Label label, int i ) { Node node = database.createNode( label ); node.setProperty( "property" + i, i ); - node.setProperty( i + "property", i ); transaction.success(); } }