Get API: a get for a document that does not exists can cause open fil…

…e handles leak, closes #1167.
elastic · Jul 26, 2011 · 995f33b · 995f33b
1 parent b34314f
commit 995f33b
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 5 deletions.
diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java b/modules/elasticsearch/src/main/java/org/elasticsearch/action/get/TransportGetAction.java
@@ -133,20 +133,28 @@ public static GetResponse load(ESLogger logger, ScriptService scriptService, Ind
                 if (get.exists()) {
                     type = typeX;
                     break;
+                } else {
+                    get.release();
                 }
             }
-            if (get == null || !get.exists()) {
+            if (get == null) {
+                return new GetResponse(index, type, id, -1, false, null, null);
+            }
+            if (!get.exists()) {
+                // no need to release here as well..., we release in the for loop for non exists
                 return new GetResponse(index, type, id, -1, false, null, null);
             }
         } else {
             get = indexShard.get(new Engine.Get(realtime, UidFieldMapper.TERM_FACTORY.createTerm(Uid.createUid(type, id))).loadSource(loadSource));
             if (!get.exists()) {
+                get.release();
                 return new GetResponse(index, type, id, -1, false, null, null);
             }
         }
 
         DocumentMapper docMapper = indexService.mapperService().documentMapper(type);
         if (docMapper == null) {
+            get.release();
             return new GetResponse(index, type, id, -1, false, null, null);
         }
 
@@ -307,9 +315,7 @@ public static GetResponse load(ESLogger logger, ScriptService scriptService, Ind
                 return new GetResponse(index, type, id, get.version(), get.exists(), sourceRequested ? source : null, fields);
             }
         } finally {
-            if (get.searcher() != null) {
-                get.searcher().release();
-            }
+            get.release();
         }
     }
 

diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/Engine.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/Engine.java
@@ -666,6 +666,12 @@ public Searcher searcher() {
         public UidField.DocIdAndVersion docIdAndVersion() {
             return docIdAndVersion;
         }
+
+        public void release() {
+            if (searcher != null) {
+                searcher.release();
+            }
+        }
     }
 
 }
diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/engine/robin/RobinEngine.java
@@ -340,7 +340,7 @@ public GetResult get(Get get) throws EngineException {
                 //TODO: A better exception goes here
                 throw new EngineException(shardId(), "failed to load document", e);
             }
-
+            searcher.release();
             return GetResult.NOT_EXISTS;
         } finally {
             rwl.readLock().unlock();