From 71eb4532f86443f2f241badbc4b7c5b4a1878fee Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 27 Nov 2013 15:55:25 +0100 Subject: [PATCH] Catch AlreadClosedException in RobinSearcher#release() When we relocate a shard we might still have pending SearchContext instances hanging around that will be used in "in-flight" searches on the already relocated shard. This is a valid operation but if we have already closed the underlying directory which happens during cleanup concurrently the close call on the IndexReader can trigger an AlreadyClosedException when the NRT reader tries to cleanup files via the IndexWriter. Closes #4273 --- .../org/elasticsearch/index/engine/robin/RobinEngine.java | 5 +++++ src/main/java/org/elasticsearch/search/SearchService.java | 6 +----- .../search/basic/SearchWhileRelocatingTests.java | 3 +-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java b/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java index a8c31294086..5a7fe939b15 100644 --- a/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java +++ b/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java @@ -1499,6 +1499,11 @@ public class RobinEngine extends AbstractIndexShardComponent implements Engine { return true; } catch (IOException e) { return false; + } catch (AlreadyClosedException e) { + /* this one can happen if we already closed the + * underlying store / directory and we call into the + * IndexWriter to free up pending files. */ + return false; } } } diff --git a/src/main/java/org/elasticsearch/search/SearchService.java b/src/main/java/org/elasticsearch/search/SearchService.java index 9e03ad6f6e2..18b69cd52c6 100644 --- a/src/main/java/org/elasticsearch/search/SearchService.java +++ b/src/main/java/org/elasticsearch/search/SearchService.java @@ -433,11 +433,7 @@ public class SearchService extends AbstractLifecycleComponent { } catch (Throwable e) { context.indexShard().searchService().onFailedFetchPhase(context); logger.trace("Fetch phase failed", e); - try { - freeContext(context); // we just try to make sure this is freed - rethrow orig exception. - } catch(Throwable t) { - logger.trace("Could not free context", t); - } + freeContext(context); // we just try to make sure this is freed - rethrow orig exception. throw ExceptionsHelper.convertToRuntime(e); } finally { cleanContext(context); diff --git a/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java b/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java index 8cb39d51c11..c0d3fa3fad1 100644 --- a/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java +++ b/src/test/java/org/elasticsearch/search/basic/SearchWhileRelocatingTests.java @@ -57,8 +57,7 @@ public class SearchWhileRelocatingTests extends ElasticsearchIntegrationTest { private void testSearchAndRelocateConcurrently(int numberOfReplicas) throws Exception { final int numShards = between(10, 20); client().admin().indices().prepareCreate("test") - .setSettings(settingsBuilder().put("index.number_of_shards", numShards).put("index.number_of_replicas", numberOfReplicas) - .put("index.store.type", "niofs")) + .setSettings(settingsBuilder().put("index.number_of_shards", numShards).put("index.number_of_replicas", numberOfReplicas)) .addMapping("type1", "loc", "type=geo_point", "test", "type=string").execute().actionGet(); ensureGreen(); List indexBuilders = new ArrayList();