GEODE-2314: EOF file segment exceptions due to empty file * We will occasionally see EOF exceptions due to file segments being empty * This diff also reinstates the query retry * Added a few null checks * Added additional logging for exceptional cases
Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/5efe89b9 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/5efe89b9 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/5efe89b9 Branch: refs/heads/feature/GEODE-2267 Commit: 5efe89b90e05b3a92afc627adfa9ba7ada485395 Parents: 77c1c6e Author: Jason Huynh <[email protected]> Authored: Fri Feb 3 11:43:31 2017 -0800 Committer: Jason Huynh <[email protected]> Committed: Tue Feb 7 09:28:07 2017 -0800 ---------------------------------------------------------------------- .../AbstractPartitionedRepositoryManager.java | 3 +- .../lucene/internal/IndexRepositoryFactory.java | 18 +++++++----- .../cache/lucene/internal/LuceneQueryImpl.java | 28 ++++++++++-------- .../internal/directory/FileIndexInput.java | 1 + .../internal/directory/RegionDirectory.java | 1 - .../cache/lucene/internal/filesystem/File.java | 2 -- .../lucene/internal/filesystem/FileSystem.java | 31 ++++++++++++++++---- 7 files changed, 54 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java index f64ca82..97acea1 100755 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java @@ -98,7 +98,6 @@ public abstract class AbstractPartitionedRepositoryManager implements Repository } catch (IOException e) { throw new InternalGemFireError("Unable to create index repository", e); } - }); return repo; } @@ -116,7 +115,7 @@ public abstract class AbstractPartitionedRepositoryManager implements Repository if (repo == null) { throw new BucketNotFoundException( - "Unable to create lucene index because no longer primary for bucket " + bucketId); + "Unable to find lucene index because no longer primary for bucket " + bucketId); } return repo; } http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java index 2b244e7..57dd0a5 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java @@ -20,9 +20,7 @@ import org.apache.geode.cache.lucene.internal.directory.RegionDirectory; import org.apache.geode.cache.lucene.internal.repository.IndexRepository; import org.apache.geode.cache.lucene.internal.repository.IndexRepositoryImpl; import org.apache.geode.cache.lucene.internal.repository.serializer.LuceneSerializer; -import org.apache.geode.cache.partition.PartitionRegionHelper; import org.apache.geode.distributed.DistributedLockService; -import org.apache.geode.internal.cache.BucketNotFoundException; import org.apache.geode.internal.cache.BucketRegion; import org.apache.geode.internal.cache.PartitionedRegion; import org.apache.geode.internal.cache.PartitionedRegionHelper; @@ -31,7 +29,6 @@ import org.apache.logging.log4j.Logger; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; -import org.jgroups.blocks.locking.LockService; public class IndexRepositoryFactory { @@ -49,11 +46,15 @@ public class IndexRepositoryFactory { BucketRegion dataBucket = getMatchingBucket(userRegion, bucketId); boolean success = false; if (fileBucket == null || chunkBucket == null) { - oldRepository.cleanup(); + if (oldRepository != null) { + oldRepository.cleanup(); + } return null; } if (!fileBucket.getBucketAdvisor().isPrimary()) { - oldRepository.cleanup(); + if (oldRepository != null) { + oldRepository.cleanup(); + } return null; } @@ -61,7 +62,7 @@ public class IndexRepositoryFactory { return oldRepository; } - if (oldRepository != null && oldRepository.isClosed()) { + if (oldRepository != null) { oldRepository.cleanup(); } DistributedLockService lockService = getLockService(); @@ -72,7 +73,6 @@ public class IndexRepositoryFactory { } } - final IndexRepository repo; try { RegionDirectory dir = @@ -83,6 +83,10 @@ public class IndexRepositoryFactory { dataBucket, lockService, lockName); success = true; return repo; + } catch (IOException e) { + logger.info("Exception thrown while constructing Lucene Index for bucket:" + bucketId + + " for file region:" + fileBucket.getFullPath()); + throw e; } finally { if (!success) { lockService.unlock(lockName); http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java index 4ac987d..b41bb5f 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java @@ -107,18 +107,22 @@ public class LuceneQueryImpl<K, V> implements LuceneQuery<K, V> { // TODO provide a timeout to the user? TopEntries<K> entries = null; - try { - TopEntriesFunctionCollector collector = new TopEntriesFunctionCollector(context); - ResultCollector<TopEntriesCollector, TopEntries<K>> rc = - (ResultCollector<TopEntriesCollector, TopEntries<K>>) onRegion().withArgs(context) - .withCollector(collector).execute(LuceneFunction.ID); - entries = rc.getResult(); - } catch (FunctionException e) { - if (e.getCause() instanceof LuceneQueryException) { - throw new LuceneQueryException(e); - } else { - e.printStackTrace(); - throw e; + while (entries == null) { + try { + TopEntriesFunctionCollector collector = new TopEntriesFunctionCollector(context); + ResultCollector<TopEntriesCollector, TopEntries<K>> rc = + (ResultCollector<TopEntriesCollector, TopEntries<K>>) onRegion().withArgs(context) + .withCollector(collector).execute(LuceneFunction.ID); + entries = rc.getResult(); + } catch (FunctionException e) { + if (e.getCause() instanceof BucketNotFoundException) { + entries = null; + } else if (e.getCause() instanceof LuceneQueryException) { + throw new LuceneQueryException(e); + } else { + e.printStackTrace(); + throw e; + } } } return entries; http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java index 4b1fc81..5fe391f 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/FileIndexInput.java @@ -24,6 +24,7 @@ import org.apache.geode.cache.lucene.internal.filesystem.File; import org.apache.geode.cache.lucene.internal.filesystem.SeekableInputStream; final class FileIndexInput extends IndexInput { + private final File file; SeekableInputStream in; private long position; http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java ---------------------------------------------------------------------- diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java index f1af7f3..2623e16 100644 --- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java +++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/directory/RegionDirectory.java @@ -109,7 +109,6 @@ public class RegionDirectory extends BaseDirectory { public IndexInput openInput(String name, IOContext context) throws IOException { ensureOpen(); final File file = fs.getFile(name); - return new FileIndexInput(name, file); } http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/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 11647f1..f3718a8 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 @@ -18,9 +18,7 @@ package org.apache.geode.cache.lucene.internal.filesystem; import java.io.DataInput; import java.io.DataOutput; import java.io.IOException; -import java.io.InputStream; import java.io.OutputStream; -import java.io.Serializable; import java.nio.file.Files; import java.util.UUID; http://git-wip-us.apache.org/repos/asf/geode/blob/5efe89b9/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 fb1913e..78a5b80 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 @@ -15,6 +15,10 @@ package org.apache.geode.cache.lucene.internal.filesystem; +import org.apache.geode.cache.Region; +import org.apache.geode.internal.logging.LogService; +import org.apache.logging.log4j.Logger; + import java.io.FileNotFoundException; import java.io.IOException; import java.util.Collection; @@ -32,7 +36,8 @@ import java.util.concurrent.ConcurrentMap; * */ public class FileSystem { - // private final Cache cache; + private static final Logger logger = LogService.getLogger(); + private final ConcurrentMap<String, File> fileRegion; private final ConcurrentMap<ChunkKey, byte[]> chunkRegion; @@ -71,6 +76,16 @@ public class FileSystem { return file; } + public File putIfAbsentFile(String name, File file) throws IOException { + // TODO lock region ? + if (null != fileRegion.putIfAbsent(name, file)) { + throw new IOException("File exists."); + } + stats.incFileCreates(1); + // TODO unlock region ? + return file; + } + public File createTemporaryFile(final String name) throws IOException { final File file = new File(this, name); stats.incTemporaryFileCreates(1); @@ -115,27 +130,27 @@ public class FileSystem { } public void renameFile(String source, String dest) throws IOException { + final File sourceFile = fileRegion.get(source); if (null == sourceFile) { throw new FileNotFoundException(source); } - final File destFile = createFile(dest); + 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; - updateFile(destFile); // 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 + putIfAbsentFile(dest, destFile); fileRegion.remove(source); - stats.incFileRenames(1); } @@ -149,12 +164,16 @@ public class FileSystem { chunkRegion.remove(key); key.chunkId++; } - return null; } final byte[] chunk = chunkRegion.get(key); - stats.incReadBytes(chunk.length); + if (chunk != null) { + stats.incReadBytes(chunk.length); + } else { + logger.debug("Chunk was null for file:" + file.getName() + " file id: " + key.getFileId() + + " chunkKey:" + key.chunkId); + } return chunk; }
