You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by up...@apache.org on 2017/02/14 20:35:25 UTC
[03/13] geode git commit: 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 w
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-2402
Commit: dbea592cc96f64c9fbc7abf8bbf597a69c5881cd
Parents: 6f35a02
Author: Jason Huynh <hu...@gmail.com>
Authored: Fri Feb 10 11:37:37 2017 -0800
Committer: Jason Huynh <hu...@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());