From 2bb99e83fa7596f6fed6d4837d99e73c9fba5fe9 Mon Sep 17 00:00:00 2001
From: Mark Robert Miller <markrmiller@apache.org>
Date: Sun, 24 Aug 2014 23:26:33 +0000
Subject: [PATCH] SOLR-6425: If you using the new global hdfs block cache
 option, you can end up reading corrupt files on file name reuse.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1620236 13f79535-47bb-0310-9956-ffa450edef68
---
 solr/CHANGES.txt                                    |  3 +++
 .../apache/solr/store/blockcache/BlockCache.java    |  2 +-
 .../solr/store/blockcache/BlockDirectory.java       |  7 +------
 .../solr/store/blockcache/BlockDirectoryCache.java  | 13 ++++++-------
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3319e46a0c1..4c8d440572a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -129,6 +129,9 @@ Bug Fixes
 * SOLR-6424: The hdfs block cache BLOCKCACHE_WRITE_ENABLED is not defaulting to false like it 
   should. (Mark Miller)
 
+* SOLR-6425: If you using the new global hdfs block cache option, you can end up reading corrupt 
+  files on file name reuse. (Mark Miller, Gregory Chanan)
+
 Other Changes
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
index 634858bf6c2..5f216691ae8 100644
--- a/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
+++ b/solr/core/src/java/org/apache/solr/store/blockcache/BlockCache.java
@@ -81,7 +81,7 @@ public class BlockCache {
   }
   
   public void release(BlockCacheKey key) {
-    releaseLocation(cache.get(key));
+    releaseLocation(cache.remove(key));
   }
   
   private void releaseLocation(BlockCacheLocation location) {
diff --git a/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectory.java b/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectory.java
index 0474b949967..97f2bbc142c 100644
--- a/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectory.java
+++ b/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectory.java
@@ -96,11 +96,6 @@ public class BlockDirectory extends Directory {
   public BlockDirectory(String dirName, Directory directory, Cache cache,
       Set<String> blockCacheFileTypes, boolean blockCacheReadEnabled,
       boolean blockCacheWriteEnabled) throws IOException {
-    this(dirName, directory, cache, blockCacheFileTypes, blockCacheReadEnabled, blockCacheWriteEnabled, false);
-  }
-  public BlockDirectory(String dirName, Directory directory, Cache cache,
-      Set<String> blockCacheFileTypes, boolean blockCacheReadEnabled,
-      boolean blockCacheWriteEnabled, boolean releaseBlocksOnClose) throws IOException {
     this.dirName = dirName;
     this.directory = directory;
     blockSize = BLOCK_SIZE;
@@ -244,11 +239,11 @@ public class BlockDirectory extends Directory {
       // segments.gen won't be removed above
       cache.delete(dirName + "/" + "segments.gen");
       
-      cache.releaseResources();
     } catch (FileNotFoundException e) {
       // the local file system folder may be gone
     } finally {
       directory.close();
+      cache.releaseResources();
     }
   }
   
diff --git a/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectoryCache.java b/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectoryCache.java
index c7e596952bb..38089eecec7 100644
--- a/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectoryCache.java
+++ b/solr/core/src/java/org/apache/solr/store/blockcache/BlockDirectoryCache.java
@@ -31,7 +31,7 @@ public class BlockDirectoryCache implements Cache {
   private final BlockCache blockCache;
   private final AtomicInteger counter = new AtomicInteger();
   private final Map<String,Integer> names = new ConcurrentHashMap<>();
-  private Set<BlockCacheKey> keys;
+  private Set<BlockCacheKey> keysToRelease;
   private final String path;
   private final Metrics metrics;
   
@@ -44,7 +44,7 @@ public class BlockDirectoryCache implements Cache {
     this.path = path;
     this.metrics = metrics;
     if (releaseBlocks) {
-      keys = Collections.synchronizedSet(new HashSet<BlockCacheKey>());
+      keysToRelease = Collections.synchronizedSet(new HashSet<BlockCacheKey>());
     }
   }
   
@@ -74,9 +74,8 @@ public class BlockDirectoryCache implements Cache {
     blockCacheKey.setPath(path);
     blockCacheKey.setBlock(blockId);
     blockCacheKey.setFile(file);
-    blockCache.store(blockCacheKey, blockOffset, buffer, offset, length);
-    if (keys != null) {
-      keys.add(blockCacheKey);
+    if (blockCache.store(blockCacheKey, blockOffset, buffer, offset, length) && keysToRelease != null) {
+      keysToRelease.add(blockCacheKey);
     }
   }
   
@@ -117,8 +116,8 @@ public class BlockDirectoryCache implements Cache {
 
   @Override
   public void releaseResources() {
-    if (keys != null) {
-      for (BlockCacheKey key : keys) {
+    if (keysToRelease != null) {
+      for (BlockCacheKey key : keysToRelease) {
         blockCache.release(key);
       }
     }