Merge remote-tracking branch 'upstream/3.4' into 3.5

community/community-it/kernel-it/src/test/java/org/neo4j/unsafe/impl/batchimport/store/BatchingNeoStoresTest.java

@@ -22,6 +22,7 @@
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.io.IOException;
 import java.util.stream.Stream;
 
 import org.neo4j.function.Predicates;
@@ -36,14 +37,15 @@
 import org.neo4j.kernel.impl.logging.NullLogService;
 import org.neo4j.kernel.impl.store.PropertyStore;
 import org.neo4j.kernel.impl.store.RecordStore;
+import org.neo4j.kernel.impl.store.RelationshipStore;
 import org.neo4j.kernel.impl.store.StoreType;
 import org.neo4j.kernel.impl.store.format.ForcedSecondaryUnitRecordFormats;
 import org.neo4j.kernel.impl.store.format.RecordFormatSelector;
 import org.neo4j.kernel.impl.store.format.RecordFormats;
-import org.neo4j.kernel.impl.store.format.standard.Standard;
 import org.neo4j.kernel.impl.store.record.AbstractBaseRecord;
 import org.neo4j.kernel.impl.store.record.PropertyBlock;
 import org.neo4j.kernel.impl.store.record.PropertyRecord;
+import org.neo4j.kernel.impl.store.record.RelationshipRecord;
 import org.neo4j.logging.NullLogProvider;
 import org.neo4j.test.TestGraphDatabaseFactory;
 import org.neo4j.test.rule.PageCacheAndDependenciesRule;
@@ -59,6 +61,8 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
+import static org.neo4j.kernel.impl.store.format.standard.Standard.LATEST_RECORD_FORMATS;
+import static org.neo4j.kernel.impl.store.record.Record.NULL_REFERENCE;
 import static org.neo4j.unsafe.impl.batchimport.AdditionalInitialIds.EMPTY;
 import static org.neo4j.unsafe.impl.batchimport.Configuration.DEFAULT;
 import static org.neo4j.unsafe.impl.batchimport.store.BatchingNeoStores.DOUBLE_RELATIONSHIP_RECORD_UNIT_THRESHOLD;
@@ -102,7 +106,7 @@ public void shouldRespectDbConfig() throws Exception
                 GraphDatabaseSettings.string_block_size.name(), String.valueOf( size ) ) );
 
         // WHEN
-        RecordFormats recordFormats = Standard.LATEST_RECORD_FORMATS;
+        RecordFormats recordFormats = LATEST_RECORD_FORMATS;
         int headerSize = recordFormats.dynamic().getRecordHeaderSize();
         try ( BatchingNeoStores store = BatchingNeoStores.batchingNeoStores( storage.fileSystem(), storage.directory().absolutePath(),
                 recordFormats, DEFAULT, NullLogService.getInstance(), EMPTY, config ) )
@@ -126,7 +130,7 @@ public void shouldPruneAndOpenExistingDatabase() throws Exception
             {
                 storage.directory().cleanup();
                 try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(), pageCache,
-                        PageCacheTracer.NULL, storage.directory().absolutePath(), Standard.LATEST_RECORD_FORMATS, DEFAULT, NullLogService.getInstance(), EMPTY,
+                        PageCacheTracer.NULL, storage.directory().absolutePath(), LATEST_RECORD_FORMATS, DEFAULT, NullLogService.getInstance(), EMPTY,
                         Config.defaults() ) )
                 {
                     stores.createNew();
@@ -138,7 +142,7 @@ public void shouldPruneAndOpenExistingDatabase() throws Exception
 
                 // when opening and pruning all except the one we test
                 try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(), pageCache,
-                        PageCacheTracer.NULL, storage.directory().absolutePath(), Standard.LATEST_RECORD_FORMATS, DEFAULT, NullLogService.getInstance(), EMPTY,
+                        PageCacheTracer.NULL, storage.directory().absolutePath(), LATEST_RECORD_FORMATS, DEFAULT, NullLogService.getInstance(), EMPTY,
                         Config.defaults() ) )
                 {
                     stores.pruneAndOpenExistingStore( type -> type == typeToTest, Predicates.alwaysFalse() );
@@ -165,7 +169,7 @@ public void shouldPruneAndOpenExistingDatabase() throws Exception
     public void shouldDecideToAllocateDoubleRelationshipRecordUnitsOnLargeAmountOfRelationshipsOnSupportedFormat() throws Exception
     {
         // given
-        RecordFormats formats = new ForcedSecondaryUnitRecordFormats( Standard.LATEST_RECORD_FORMATS );
+        RecordFormats formats = new ForcedSecondaryUnitRecordFormats( LATEST_RECORD_FORMATS );
         try ( PageCache pageCache = storage.pageCache();
               BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(),
                 pageCache, PageCacheTracer.NULL, storage.directory().absolutePath(), formats, DEFAULT,
@@ -186,7 +190,7 @@ public void shouldDecideToAllocateDoubleRelationshipRecordUnitsOnLargeAmountOfRe
     public void shouldNotDecideToAllocateDoubleRelationshipRecordUnitsonLowAmountOfRelationshipsOnSupportedFormat() throws Exception
     {
         // given
-        RecordFormats formats = new ForcedSecondaryUnitRecordFormats( Standard.LATEST_RECORD_FORMATS );
+        RecordFormats formats = new ForcedSecondaryUnitRecordFormats( LATEST_RECORD_FORMATS );
         try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(),
                 storage.pageCache(), PageCacheTracer.NULL, storage.directory().absolutePath(), formats, DEFAULT,
                 NullLogService.getInstance(), EMPTY, Config.defaults() ) )
@@ -206,7 +210,7 @@ public void shouldNotDecideToAllocateDoubleRelationshipRecordUnitsonLowAmountOfR
     public void shouldNotDecideToAllocateDoubleRelationshipRecordUnitsonLargeAmountOfRelationshipsOnUnsupportedFormat() throws Exception
     {
         // given
-        RecordFormats formats = Standard.LATEST_RECORD_FORMATS;
+        RecordFormats formats = LATEST_RECORD_FORMATS;
         try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(),
                 storage.pageCache(), PageCacheTracer.NULL, storage.directory().absolutePath(), formats, DEFAULT,
                 NullLogService.getInstance(), EMPTY, Config.defaults() ) )
@@ -222,6 +226,39 @@ public void shouldNotDecideToAllocateDoubleRelationshipRecordUnitsonLargeAmountO
         }
     }
 
+    @Test
+    public void shouldDeleteIdGeneratorsWhenOpeningExistingStore() throws IOException
+    {
+        // given
+        long expectedHighId;
+        try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(),
+                storage.pageCache(), PageCacheTracer.NULL, storage.directory().absolutePath(), LATEST_RECORD_FORMATS, DEFAULT,
+                NullLogService.getInstance(), EMPTY, Config.defaults() ) )
+        {
+            stores.createNew();
+            RelationshipStore relationshipStore = stores.getRelationshipStore();
+            RelationshipRecord record = relationshipStore.newRecord();
+            long no = NULL_REFERENCE.longValue();
+            record.initialize( true, no, 1, 2, 0, no, no, no, no, true, true );
+            record.setId( relationshipStore.nextId() );
+            expectedHighId = relationshipStore.getHighId();
+            relationshipStore.updateRecord( record );
+            // fiddle with the highId
+            relationshipStore.setHighId( record.getId() + 999 );
+        }
+
+        // when
+        try ( BatchingNeoStores stores = BatchingNeoStores.batchingNeoStoresWithExternalPageCache( storage.fileSystem(),
+                storage.pageCache(), PageCacheTracer.NULL, storage.directory().absolutePath(), LATEST_RECORD_FORMATS, DEFAULT,
+                NullLogService.getInstance(), EMPTY, Config.defaults() ) )
+        {
+            stores.pruneAndOpenExistingStore( Predicates.alwaysTrue(), Predicates.alwaysTrue() );
+
+            // then
+            assertEquals( expectedHighId, stores.getRelationshipStore().getHighId() );
+        }
+    }
+
     private StoreType[] relevantRecordStores()
     {
         return Stream.of( StoreType.values() )

community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/store/BatchingNeoStores.java

@@ -242,6 +242,14 @@ private void instantiateStores()
         temporaryNeoStores = instantiateTempStores();
         instantiateKernelExtensions();
 
+        // Delete the id generators because makeStoreOk isn't atomic in the sense that there's a possibility of an unlucky timing such
+        // that if the process is killed at the right time some store may end up with a .id file that looks to be CLEAN and has highId=0,
+        // i.e. effectively making the store look empty on the next start. Normal recovery of a db is sort of protected by this recovery
+        // recognizing that the db needs recovery when it looks at the tx log and also calling deleteIdGenerators. In the import case
+        // there are no tx logs at all, and therefore we do this manually right here.
+        neoStores.deleteIdGenerators();
+        temporaryNeoStores.deleteIdGenerators();
+
         neoStores.makeStoreOk();
         temporaryNeoStores.makeStoreOk();
     }