Repository: geode Updated Branches: refs/heads/feature/GEODE-2449 5562547e0 -> 2fbdb0cd8
GEODE-2459: When node fails while renaming, new primary node resumes and may delete chunks * Rename will set a flag on the renamed file before continuing * Deleting possibly renamed files will not delete chunks * This means we may have lingering chunks if a node crashes but we should no longer see EOF exceptions due to missing chunks Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/dbea592c Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/dbea592c Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/dbea592c Branch: refs/heads/feature/GEODE-2449 Commit: dbea592cc96f64c9fbc7abf8bbf597a69c5881cd Parents: 6f35a02 Author: Jason Huynh <huyn...@gmail.com> Authored: Fri Feb 10 11:37:37 2017 -0800 Committer: Jason Huynh <huyn...@gmail.com> Committed: Fri Feb 10 13:00:30 2017 -0800 ---------------------------------------------------------------------- .../cache/lucene/internal/filesystem/File.java | 3 ++ .../lucene/internal/filesystem/FileSystem.java | 23 +++++++------ .../filesystem/FileSystemJUnitTest.java | 35 ++++++++++++++++---- 3 files changed, 44 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/dbea592c/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java index f3718a8..23db07d 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/File.java @@ -41,6 +41,7 @@ public class File implements DataSerializableFixedID { long created = System.currentTimeMillis(); long modified = created; UUID id = UUID.randomUUID(); + boolean possiblyRenamed = false; /** * Constructor for serialization only @@ -130,6 +131,7 @@ public class File implements DataSerializableFixedID { out.writeLong(modified); out.writeLong(id.getMostSignificantBits()); out.writeLong(id.getLeastSignificantBits()); + out.writeBoolean(possiblyRenamed); } @Override @@ -142,6 +144,7 @@ public class File implements DataSerializableFixedID { long high = in.readLong(); long low = in.readLong(); id = new UUID(high, low); + possiblyRenamed = in.readBoolean(); } http://git-wip-us.apache.org/repos/asf/geode/blob/dbea592c/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java index 78a5b80..f3975bf 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystem.java @@ -115,17 +115,18 @@ public class FileSystem { throw new FileNotFoundException(name); } - // TODO consider removeAll with all ChunkKeys listed. - final ChunkKey key = new ChunkKey(file.id, 0); - while (true) { - // TODO consider mutable ChunkKey - if (null == chunkRegion.remove(key)) { - // no more chunks - break; + if (file.possiblyRenamed == false) { + // TODO consider removeAll with all ChunkKeys listed. + final ChunkKey key = new ChunkKey(file.id, 0); + while (true) { + // TODO consider mutable ChunkKey + if (null == chunkRegion.remove(key)) { + // no more chunks + break; + } + key.chunkId++; } - key.chunkId++; } - stats.incFileDeletes(1); } @@ -137,17 +138,17 @@ public class FileSystem { } final File destFile = new File(this, dest); - destFile.chunks = sourceFile.chunks; destFile.created = sourceFile.created; destFile.length = sourceFile.length; destFile.modified = sourceFile.modified; destFile.id = sourceFile.id; - + sourceFile.possiblyRenamed = true; // TODO - What is the state of the system if // things crash in the middle of moving this file? // Seems like we will have two files pointing // at the same data + updateFile(sourceFile); putIfAbsentFile(dest, destFile); fileRegion.remove(source); http://git-wip-us.apache.org/repos/asf/geode/blob/dbea592c/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java index b10b32a..ee41e40 100644 --- a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java +++ b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/filesystem/FileSystemJUnitTest.java @@ -17,7 +17,6 @@ package org.apache.geode.cache.lucene.internal.filesystem; import static org.junit.Assert.*; import static org.mockito.Mockito.*; -import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; @@ -30,14 +29,13 @@ import java.util.HashSet; import java.util.Random; import java.util.concurrent.ConcurrentHashMap; -import org.apache.commons.io.FileUtils; -import org.apache.commons.io.IOUtils; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; +import org.mockito.Spy; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -373,18 +371,22 @@ public class FileSystemJUnitTest { system.renameFile(name, name2); - assertTrue(2 <= countOperations.count); + // Right now the number of operations is 4.. except if run through a debugger... + assertTrue(4 <= countOperations.count); - countOperations.after((int) Math.ceil(countOperations.count / 2.0), new Runnable() { + // This number of operations during a rename actually needs to get to the "putIfAbsent" for the + // Assertion to be correct. Right now the number of operations is actually 3 so the limit needs + // to be 3... + countOperations.after((int) Math.ceil(countOperations.count / 2.0 + 1), new Runnable() { @Override public void run() { throw new CacheClosedException(); } }); + String name3 = "file3"; countOperations.reset(); - String name3 = "file3"; try { system.renameFile(name2, name3); fail("should have seen an error"); @@ -540,6 +542,27 @@ public class FileSystemJUnitTest { assertEquals(bytes.length, actualByteCount); } + @Test + public void testDeletePossiblyRenamedFileDoesNotDestroyChunks() throws Exception { + ConcurrentHashMap<String, File> spyFileRegion = Mockito.spy(fileRegion); + system = new FileSystem(spyFileRegion, chunkRegion, fileSystemStats); + + String sourceFileName = "sourceFile"; + File file1 = system.createFile(sourceFileName); + byte[] data = writeRandomBytes(file1); + + Mockito.doReturn(file1).when(spyFileRegion).remove(any()); + + String destFileName = "destFile"; + system.renameFile(sourceFileName, destFileName); + File destFile = system.getFile(destFileName); + + assertNotNull(system.getFile(sourceFileName)); + system.deleteFile(sourceFileName); + assertNotNull(system.getChunk(destFile, 0)); + + } + private void assertExportedFileContents(final byte[] expected, final java.io.File exportedFile) throws IOException { byte[] actual = Files.readAllBytes(exportedFile.toPath());
