From 19c97f44baa993810963bcff8e36f39efc621690 Mon Sep 17 00:00:00 2001 From: laa Date: Wed, 19 Jul 2017 10:46:08 +0300 Subject: [PATCH 1/2] Issue #7546 was fixed. --- core/pom.xml | 4 + .../core/storage/cache/local/OWOWCache.java | 45 ++++++ .../paginated/InvalidRemovedFileIdsTest.java | 144 ++++++++++++++++++ 3 files changed, 193 insertions(+) mode change 100644 => 100755 core/pom.xml create mode 100755 core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java diff --git a/core/pom.xml b/core/pom.xml old mode 100644 new mode 100755 index 9b216012be8..0118111922b --- a/core/pom.xml +++ b/core/pom.xml @@ -82,6 +82,7 @@ **/SBTreeWALTest.java **/LocalPaginatedClusterWithWALTest.java **/OSBTreeBonsaiWALTest.java + **/InvalidRemovedFileIdsTest.java @@ -105,6 +106,7 @@ empty.java empty.java empty.java + empty.java @@ -128,6 +130,7 @@ empty.java empty.java empty.java + empty.java @@ -201,6 +204,7 @@ ${exclude.test.9} ${exclude.test.10} ${exclude.test.11} + ${exclude.test.12} diff --git a/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java b/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java index a8fdcf871be..79f9532c4c0 100755 --- a/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java +++ b/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java @@ -29,6 +29,7 @@ import com.orientechnologies.common.serialization.types.OIntegerSerializer; import com.orientechnologies.common.serialization.types.OLongSerializer; import com.orientechnologies.common.types.OModifiableBoolean; +import com.orientechnologies.common.util.OPair; import com.orientechnologies.orient.core.command.OCommandOutputListener; import com.orientechnologies.orient.core.config.OGlobalConfiguration; import com.orientechnologies.orient.core.exception.OStorageException; @@ -1267,6 +1268,12 @@ private OFileClassic createFileInstance(String fileName) throws InterruptedExcep } private void readNameIdMap() throws IOException, InterruptedException { + //older versions of ODB incorrectly logged file deletions + //some deleted files have the same id + //because we reuse ids of removed files when we re-create them + //we need to fix this situation + final Map> filesWithfNegativeIds = new HashMap>(); + nameIdMap = new ConcurrentHashMap(); long localFileCounter = -1; @@ -1279,6 +1286,32 @@ private void readNameIdMap() throws IOException, InterruptedException { if (localFileCounter < absFileId) localFileCounter = absFileId; + final Integer existingId = nameIdMap.get(nameFileIdEntry.name); + + if (existingId != null && existingId < 0) { + final Set files = filesWithfNegativeIds.get(existingId); + + if (files != null) { + files.remove(nameFileIdEntry.name); + + if (files.isEmpty()) { + filesWithfNegativeIds.remove(existingId); + } + } + } + + if (nameFileIdEntry.fileId < 0) { + Set files = filesWithfNegativeIds.get(nameFileIdEntry.fileId); + + if (files == null) { + files = new HashSet(); + files.add(nameFileIdEntry.name); + filesWithfNegativeIds.put(nameFileIdEntry.fileId, files); + } else { + files.add(nameFileIdEntry.name); + } + } + nameIdMap.put(nameFileIdEntry.name, nameFileIdEntry.fileId); } @@ -1305,6 +1338,18 @@ private void readNameIdMap() throws IOException, InterruptedException { } } } + + for (Map.Entry> entry : filesWithfNegativeIds.entrySet()) { + final Set files = entry.getValue(); + + if (files.size() > 1) { + for (String fileName : files) { + fileCounter++; + final int nextId = -fileCounter; + nameIdMap.put(fileName, nextId); + } + } + } } private NameFileIdEntry readNextNameIdEntry() throws IOException { diff --git a/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java b/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java new file mode 100755 index 00000000000..b8e37696b59 --- /dev/null +++ b/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java @@ -0,0 +1,144 @@ +package com.orientechnologies.orient.core.storage.impl.local.paginated; + +import com.orientechnologies.common.serialization.types.OIntegerSerializer; +import com.orientechnologies.common.serialization.types.OLongSerializer; +import com.orientechnologies.common.serialization.types.OStringSerializer; +import com.orientechnologies.orient.core.db.ODatabase; +import com.orientechnologies.orient.core.db.document.ODatabaseDocumentTx; +import com.orientechnologies.orient.core.metadata.schema.OSchema; +import com.orientechnologies.orient.core.storage.OStorage; +import com.orientechnologies.orient.core.storage.cache.OWriteCache; +import com.orientechnologies.orient.core.storage.impl.local.OAbstractPaginatedStorage; +import com.sun.xml.internal.ws.policy.AssertionSet; +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.io.File; +import java.io.IOException; +import java.io.RandomAccessFile; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +@Test +public class InvalidRemovedFileIdsTest { + + public void testRemovedFileIds() throws Exception { + final String buildDirectory = System.getProperty("buildDirectory", "."); + final String dbPath = buildDirectory + File.separator + InvalidRemovedFileIdsTest.class.getSimpleName(); + + deleteDirectory(new File(dbPath)); + + ODatabaseDocumentTx db = new ODatabaseDocumentTx("plocal:" + dbPath); + db.create(); + + OStorage storage = db.getStorage(); + db.close(); + storage.close(true, false); + + final RandomAccessFile cacheState = new RandomAccessFile(new File(dbPath, "name_id_map.cm"), "rw"); + cacheState.seek(cacheState.length()); + + writeNameIdEntry(cacheState, "c1.cpm", -100); + writeNameIdEntry(cacheState, "c1.pcl", -100); + + writeNameIdEntry(cacheState, "c2.cpm", -200); + writeNameIdEntry(cacheState, "c2.pcl", -200); + writeNameIdEntry(cacheState, "c2.pcl", -400); + + writeNameIdEntry(cacheState, "c3.cpm", -500); + writeNameIdEntry(cacheState, "c3.pcl", -500); + writeNameIdEntry(cacheState, "c4.cpm", -500); + writeNameIdEntry(cacheState, "c4.pcl", -600); + writeNameIdEntry(cacheState, "c4.cpm", -600); + + db = new ODatabaseDocumentTx("plocal:" + dbPath); + db.open("admin", "admin"); + + db.set(ODatabase.ATTRIBUTES.MINIMUMCLUSTERS, 1); + + final OSchema schema = db.getMetadata().getSchema(); + schema.createClass("c1"); + schema.createClass("c2"); + schema.createClass("c3"); + schema.createClass("c4"); + + storage = db.getStorage(); + OWriteCache writeCache = ((OAbstractPaginatedStorage) storage).getWriteCache(); + + final Map files = writeCache.files(); + final Set ids = new HashSet(); + + final Long c1_cpm_id = files.get("c1.cpm"); + Assert.assertNotNull(c1_cpm_id); + Assert.assertTrue(c1_cpm_id > 0); + Assert.assertTrue(ids.add(c1_cpm_id)); + + final Long c1_pcl_id = files.get("c1.pcl"); + Assert.assertNotNull(c1_pcl_id); + Assert.assertTrue(c1_pcl_id > 0); + Assert.assertTrue(ids.add(c1_pcl_id)); + + final Long c2_cpm_id = files.get("c2.cpm"); + Assert.assertNotNull(c2_cpm_id); + Assert.assertTrue(ids.add(c2_cpm_id)); + Assert.assertEquals(writeCache.internalFileId(c2_cpm_id), 200); //check that updated file map has been read + + final Long c2_pcl_id = files.get("c2.pcl"); + Assert.assertNotNull(c2_pcl_id); + Assert.assertTrue(ids.add(c2_pcl_id)); + Assert.assertEquals(writeCache.internalFileId(c2_pcl_id), 400); //check that updated file map has been read + + final Long c3_cpm_id = files.get("c3.cpm"); + Assert.assertNotNull(c3_cpm_id); + Assert.assertTrue(c3_cpm_id > 0); + Assert.assertTrue(ids.add(c3_cpm_id)); + + final Long c3_pcl_id = files.get("c3.pcl"); + Assert.assertNotNull(c3_pcl_id); + Assert.assertTrue(c3_pcl_id > 0); + Assert.assertTrue(ids.add(c3_pcl_id)); + + final Long c4_cpm_id = files.get("c4.cpm"); + Assert.assertNotNull(c4_cpm_id); + Assert.assertTrue(c4_cpm_id > 0); + Assert.assertTrue(ids.add(c4_cpm_id)); + + final Long c4_pcl_id = files.get("c4.pcl"); + Assert.assertNotNull(c4_pcl_id); + Assert.assertTrue(c1_pcl_id > 0); + Assert.assertTrue(ids.add(c4_pcl_id)); + + db.close(); + } + + private static void writeNameIdEntry(RandomAccessFile file, String name, int fileId) throws IOException { + final int nameSize = OStringSerializer.INSTANCE.getObjectSize(name); + + byte[] serializedRecord = new byte[OIntegerSerializer.INT_SIZE + nameSize + OLongSerializer.LONG_SIZE]; + OIntegerSerializer.INSTANCE.serializeLiteral(nameSize, serializedRecord, 0); + OStringSerializer.INSTANCE.serialize(name, serializedRecord, OIntegerSerializer.INT_SIZE); + OLongSerializer.INSTANCE.serializeLiteral(fileId, serializedRecord, OIntegerSerializer.INT_SIZE + nameSize); + + file.write(serializedRecord); + } + + private static void deleteDirectory(final File directory) { + if (directory.exists()) { + final File[] files = directory.listFiles(); + + if (files != null) { + for (File file : files) { + if (file.isDirectory()) { + deleteDirectory(file); + } else { + Assert.assertTrue(file.delete()); + } + } + + Assert.assertTrue(directory.delete()); + } + + } + } +} From 89c1af2cbb5fa5dcbc0cd0b61475f436756da9f8 Mon Sep 17 00:00:00 2001 From: laa Date: Wed, 19 Jul 2017 12:00:37 +0300 Subject: [PATCH 2/2] Issue #7546 - minor refactoring --- .../core/storage/cache/local/OWOWCache.java | 8 +++++- .../paginated/InvalidRemovedFileIdsTest.java | 25 +++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java b/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java index 79f9532c4c0..3d19e5f8bb3 100755 --- a/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java +++ b/core/src/main/java/com/orientechnologies/orient/core/storage/cache/local/OWOWCache.java @@ -29,7 +29,6 @@ import com.orientechnologies.common.serialization.types.OIntegerSerializer; import com.orientechnologies.common.serialization.types.OLongSerializer; import com.orientechnologies.common.types.OModifiableBoolean; -import com.orientechnologies.common.util.OPair; import com.orientechnologies.orient.core.command.OCommandOutputListener; import com.orientechnologies.orient.core.config.OGlobalConfiguration; import com.orientechnologies.orient.core.exception.OStorageException; @@ -1339,6 +1338,8 @@ private void readNameIdMap() throws IOException, InterruptedException { } } + final Set fixedFiles = new HashSet(); + for (Map.Entry> entry : filesWithfNegativeIds.entrySet()) { final Set files = entry.getValue(); @@ -1347,9 +1348,14 @@ private void readNameIdMap() throws IOException, InterruptedException { fileCounter++; final int nextId = -fileCounter; nameIdMap.put(fileName, nextId); + + fixedFiles.add(fileName); } } } + + if (!fixedFiles.isEmpty()) + OLogManager.instance().warn(this, "Removed files " + fixedFiles + " had duplicated ids. Problem is fixed automatically."); } private NameFileIdEntry readNextNameIdEntry() throws IOException { diff --git a/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java b/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java index b8e37696b59..2114cce8c7c 100755 --- a/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java +++ b/core/src/test/java/com/orientechnologies/orient/core/storage/impl/local/paginated/InvalidRemovedFileIdsTest.java @@ -9,7 +9,6 @@ import com.orientechnologies.orient.core.storage.OStorage; import com.orientechnologies.orient.core.storage.cache.OWriteCache; import com.orientechnologies.orient.core.storage.impl.local.OAbstractPaginatedStorage; -import com.sun.xml.internal.ws.policy.AssertionSet; import org.testng.Assert; import org.testng.annotations.Test; @@ -36,21 +35,21 @@ public void testRemovedFileIds() throws Exception { db.close(); storage.close(true, false); - final RandomAccessFile cacheState = new RandomAccessFile(new File(dbPath, "name_id_map.cm"), "rw"); - cacheState.seek(cacheState.length()); + final RandomAccessFile fileMap = new RandomAccessFile(new File(dbPath, "name_id_map.cm"), "rw"); + fileMap.seek(fileMap.length()); - writeNameIdEntry(cacheState, "c1.cpm", -100); - writeNameIdEntry(cacheState, "c1.pcl", -100); + writeNameIdEntry(fileMap, "c1.cpm", -100); + writeNameIdEntry(fileMap, "c1.pcl", -100); - writeNameIdEntry(cacheState, "c2.cpm", -200); - writeNameIdEntry(cacheState, "c2.pcl", -200); - writeNameIdEntry(cacheState, "c2.pcl", -400); + writeNameIdEntry(fileMap, "c2.cpm", -200); + writeNameIdEntry(fileMap, "c2.pcl", -200); + writeNameIdEntry(fileMap, "c2.pcl", -400); - writeNameIdEntry(cacheState, "c3.cpm", -500); - writeNameIdEntry(cacheState, "c3.pcl", -500); - writeNameIdEntry(cacheState, "c4.cpm", -500); - writeNameIdEntry(cacheState, "c4.pcl", -600); - writeNameIdEntry(cacheState, "c4.cpm", -600); + writeNameIdEntry(fileMap, "c3.cpm", -500); + writeNameIdEntry(fileMap, "c3.pcl", -500); + writeNameIdEntry(fileMap, "c4.cpm", -500); + writeNameIdEntry(fileMap, "c4.pcl", -600); + writeNameIdEntry(fileMap, "c4.cpm", -600); db = new ODatabaseDocumentTx("plocal:" + dbPath); db.open("admin", "admin");