From ad392bb11e82043259967305456d2a0c9886d02f Mon Sep 17 00:00:00 2001
From: Shay Banon <kimchy@gmail.com>
Date: Fri, 3 Aug 2012 18:40:49 +0300
Subject: [PATCH] Improve recovery time when processing large mappings, closes
 #2138.

---
 .../index/mapper/DocumentMapper.java          |  22 ++-
 .../index/mapper/FieldMapperListener.java     |  13 ++
 .../index/mapper/MapperService.java           | 128 +++++++++++-------
 .../index/mapper/ObjectMapperListener.java    |  13 ++
 4 files changed, 122 insertions(+), 54 deletions(-)

diff --git a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
index c3e67658823..c05e2aea4c2 100644
--- a/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
+++ b/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java
@@ -536,15 +536,19 @@ public class DocumentMapper implements ToXContent {
     public void addFieldMapperListener(FieldMapperListener fieldMapperListener, boolean includeExisting) {
         fieldMapperListeners.add(fieldMapperListener);
         if (includeExisting) {
-            for (RootMapper rootMapper : rootMappersOrdered) {
-                if (!rootMapper.includeInObject() && rootMapper instanceof FieldMapper) {
-                    fieldMapperListener.fieldMapper((FieldMapper) rootMapper);
-                }
-            }
-            rootObjectMapper.traverse(fieldMapperListener);
+            traverse(fieldMapperListener);
         }
     }
 
+    public void traverse(FieldMapperListener listener) {
+        for (RootMapper rootMapper : rootMappersOrdered) {
+            if (!rootMapper.includeInObject() && rootMapper instanceof FieldMapper) {
+                listener.fieldMapper((FieldMapper) rootMapper);
+            }
+        }
+        rootObjectMapper.traverse(listener);
+    }
+
     public void addObjectMapper(ObjectMapper objectMapper) {
         synchronized (mutex) {
             objectMappers = MapBuilder.newMapBuilder(objectMappers).put(objectMapper.fullPath(), objectMapper).immutableMap();
@@ -560,10 +564,14 @@ public class DocumentMapper implements ToXContent {
     public void addObjectMapperListener(ObjectMapperListener objectMapperListener, boolean includeExisting) {
         objectMapperListeners.add(objectMapperListener);
         if (includeExisting) {
-            rootObjectMapper.traverse(objectMapperListener);
+            traverse(objectMapperListener);
         }
     }
 
+    public void traverse(ObjectMapperListener listener) {
+        rootObjectMapper.traverse(listener);
+    }
+
     public synchronized MergeResult merge(DocumentMapper mergeWith, MergeFlags mergeFlags) {
         MergeContext mergeContext = new MergeContext(this, mergeFlags);
         rootObjectMapper.merge(mergeWith.rootObjectMapper, mergeContext);
diff --git a/src/main/java/org/elasticsearch/index/mapper/FieldMapperListener.java b/src/main/java/org/elasticsearch/index/mapper/FieldMapperListener.java
index 3f7fbc41ad1..a198f679707 100644
--- a/src/main/java/org/elasticsearch/index/mapper/FieldMapperListener.java
+++ b/src/main/java/org/elasticsearch/index/mapper/FieldMapperListener.java
@@ -19,10 +19,23 @@
 
 package org.elasticsearch.index.mapper;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  *
  */
 public interface FieldMapperListener {
 
+    public static class Aggregator implements FieldMapperListener {
+
+        public final List<FieldMapper> fieldMappers = new ArrayList<FieldMapper>();
+
+        @Override
+        public void fieldMapper(FieldMapper fieldMapper) {
+            fieldMappers.add(fieldMapper);
+        }
+    }
+
     void fieldMapper(FieldMapper fieldMapper);
 }
diff --git a/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/src/main/java/org/elasticsearch/index/mapper/MapperService.java
index 8207a251ac7..be9b54a3ab2 100644
--- a/src/main/java/org/elasticsearch/index/mapper/MapperService.java
+++ b/src/main/java/org/elasticsearch/index/mapper/MapperService.java
@@ -33,6 +33,7 @@ import org.apache.lucene.search.Filter;
 import org.apache.lucene.search.FilterClause;
 import org.apache.lucene.search.XTermsFilter;
 import org.elasticsearch.common.Nullable;
+import org.elasticsearch.common.collect.MapBuilder;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.io.Streams;
 import org.elasticsearch.common.lucene.search.TermFilter;
@@ -86,7 +87,7 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
     private volatile Map<String, FieldMappers> nameFieldMappers = ImmutableMap.of();
     private volatile Map<String, FieldMappers> indexNameFieldMappers = ImmutableMap.of();
     private volatile Map<String, FieldMappers> fullNameFieldMappers = ImmutableMap.of();
-    private volatile Map<String, ObjectMappers> objectMappers = ImmutableMap.of();
+    private volatile Map<String, ObjectMappers> fullPathObjectMappers = ImmutableMap.of();
     private boolean hasNested = false; // updated dynamically to true when a nested object is added
 
     private final DocumentMapperParser documentParser;
@@ -206,16 +207,84 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
             // since we get new instances of those, and when we remove, we remove
             // by instance equality
             DocumentMapper oldMapper = mappers.get(mapper.type());
-            mapper.addFieldMapperListener(fieldMapperListener, true);
-            mapper.addObjectMapperListener(objectMapperListener, true);
+
+            FieldMapperListener.Aggregator fieldMappersAgg = new FieldMapperListener.Aggregator();
+            mapper.traverse(fieldMappersAgg);
+            addFieldMappers(fieldMappersAgg.fieldMappers.toArray(new FieldMapper[fieldMappersAgg.fieldMappers.size()]));
+            mapper.addFieldMapperListener(fieldMapperListener, false);
+
+            ObjectMapperListener.Aggregator objectMappersAgg = new ObjectMapperListener.Aggregator();
+            mapper.traverse(objectMappersAgg);
+            addObjectMappers(objectMappersAgg.objectMappers.toArray(new ObjectMapper[objectMappersAgg.objectMappers.size()]));
+            mapper.addObjectMapperListener(objectMapperListener, false);
+
             mappers = newMapBuilder(mappers).put(mapper.type(), mapper).map();
             if (oldMapper != null) {
-                removeObjectFieldMappers(oldMapper);
+                removeObjectAndFieldMappers(oldMapper);
                 oldMapper.close();
             }
         }
     }
 
+    private void addObjectMappers(ObjectMapper[] objectMappers) {
+        synchronized (mutex) {
+            MapBuilder<String, ObjectMappers> fullPathObjectMappers = newMapBuilder(this.fullPathObjectMappers);
+            for (ObjectMapper objectMapper : objectMappers) {
+                ObjectMappers mappers = fullPathObjectMappers.get(objectMapper.fullPath());
+                if (mappers == null) {
+                    mappers = new ObjectMappers(objectMapper);
+                } else {
+                    mappers = mappers.concat(objectMapper);
+                }
+                fullPathObjectMappers.put(objectMapper.fullPath(), mappers);
+                // update the hasNested flag
+                if (objectMapper.nested().isNested()) {
+                    hasNested = true;
+                }
+            }
+            this.fullPathObjectMappers = fullPathObjectMappers.map();
+        }
+    }
+
+    private void addFieldMappers(FieldMapper[] fieldMappers) {
+        synchronized (mutex) {
+            MapBuilder<String, FieldMappers> nameFieldMappers = newMapBuilder(this.nameFieldMappers);
+            MapBuilder<String, FieldMappers> indexNameFieldMappers = newMapBuilder(this.indexNameFieldMappers);
+            MapBuilder<String, FieldMappers> fullNameFieldMappers = newMapBuilder(this.fullNameFieldMappers);
+            for (FieldMapper fieldMapper : fieldMappers) {
+                FieldMappers mappers = nameFieldMappers.get(fieldMapper.names().name());
+                if (mappers == null) {
+                    mappers = new FieldMappers(fieldMapper);
+                } else {
+                    mappers = mappers.concat(fieldMapper);
+                }
+                nameFieldMappers.put(fieldMapper.names().name(), mappers);
+
+
+                mappers = indexNameFieldMappers.get(fieldMapper.names().indexName());
+                if (mappers == null) {
+                    mappers = new FieldMappers(fieldMapper);
+                } else {
+                    mappers = mappers.concat(fieldMapper);
+                }
+                indexNameFieldMappers.put(fieldMapper.names().indexName(), mappers);
+
+
+                mappers = fullNameFieldMappers.get(fieldMapper.names().fullName());
+                if (mappers == null) {
+                    mappers = new FieldMappers(fieldMapper);
+                } else {
+                    mappers = mappers.concat(fieldMapper);
+                }
+                fullNameFieldMappers.put(fieldMapper.names().fullName(), mappers);
+            }
+
+            this.nameFieldMappers = nameFieldMappers.map();
+            this.indexNameFieldMappers = indexNameFieldMappers.map();
+            this.fullNameFieldMappers = fullNameFieldMappers.map();
+        }
+    }
+
     public void remove(String type) {
         synchronized (mutex) {
             DocumentMapper docMapper = mappers.get(type);
@@ -224,11 +293,11 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
             }
             docMapper.close();
             mappers = newMapBuilder(mappers).remove(type).map();
-            removeObjectFieldMappers(docMapper);
+            removeObjectAndFieldMappers(docMapper);
         }
     }
 
-    private void removeObjectFieldMappers(DocumentMapper docMapper) {
+    private void removeObjectAndFieldMappers(DocumentMapper docMapper) {
         // we need to remove those mappers
         for (FieldMapper mapper : docMapper.mappers()) {
             FieldMappers mappers = nameFieldMappers.get(mapper.names().name());
@@ -263,13 +332,13 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
         }
 
         for (ObjectMapper mapper : docMapper.objectMappers().values()) {
-            ObjectMappers mappers = objectMappers.get(mapper.fullPath());
+            ObjectMappers mappers = fullPathObjectMappers.get(mapper.fullPath());
             if (mappers != null) {
                 mappers = mappers.remove(mapper);
                 if (mappers.isEmpty()) {
-                    objectMappers = newMapBuilder(objectMappers).remove(mapper.fullPath()).map();
+                    fullPathObjectMappers = newMapBuilder(fullPathObjectMappers).remove(mapper.fullPath()).map();
                 } else {
-                    objectMappers = newMapBuilder(objectMappers).put(mapper.fullPath(), mappers).map();
+                    fullPathObjectMappers = newMapBuilder(fullPathObjectMappers).put(mapper.fullPath(), mappers).map();
                 }
             }
         }
@@ -403,7 +472,7 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
      * Returns objects mappers based on the full path of the object.
      */
     public ObjectMappers objectMapper(String path) {
-        return objectMappers.get(path);
+        return fullPathObjectMappers.get(path);
     }
 
     public Set<String> simpleMatchToIndexNames(String pattern) {
@@ -977,49 +1046,14 @@ public class MapperService extends AbstractIndexComponent implements Iterable<Do
     class InternalFieldMapperListener implements FieldMapperListener {
         @Override
         public void fieldMapper(FieldMapper fieldMapper) {
-            synchronized (mutex) {
-                FieldMappers mappers = nameFieldMappers.get(fieldMapper.names().name());
-                if (mappers == null) {
-                    mappers = new FieldMappers(fieldMapper);
-                } else {
-                    mappers = mappers.concat(fieldMapper);
-                }
-
-                nameFieldMappers = newMapBuilder(nameFieldMappers).put(fieldMapper.names().name(), mappers).map();
-
-                mappers = indexNameFieldMappers.get(fieldMapper.names().indexName());
-                if (mappers == null) {
-                    mappers = new FieldMappers(fieldMapper);
-                } else {
-                    mappers = mappers.concat(fieldMapper);
-                }
-                indexNameFieldMappers = newMapBuilder(indexNameFieldMappers).put(fieldMapper.names().indexName(), mappers).map();
-
-                mappers = fullNameFieldMappers.get(fieldMapper.names().fullName());
-                if (mappers == null) {
-                    mappers = new FieldMappers(fieldMapper);
-                } else {
-                    mappers = mappers.concat(fieldMapper);
-                }
-                fullNameFieldMappers = newMapBuilder(fullNameFieldMappers).put(fieldMapper.names().fullName(), mappers).map();
-            }
+            addFieldMappers(new FieldMapper[]{fieldMapper});
         }
     }
 
     class InternalObjectMapperListener implements ObjectMapperListener {
         @Override
         public void objectMapper(ObjectMapper objectMapper) {
-            ObjectMappers mappers = objectMappers.get(objectMapper.fullPath());
-            if (mappers == null) {
-                mappers = new ObjectMappers(objectMapper);
-            } else {
-                mappers = mappers.concat(objectMapper);
-            }
-            objectMappers = newMapBuilder(objectMappers).put(objectMapper.fullPath(), mappers).map();
-            // update the hasNested flag
-            if (objectMapper.nested().isNested()) {
-                hasNested = true;
-            }
+            addObjectMappers(new ObjectMapper[]{objectMapper});
         }
     }
 }
diff --git a/src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java b/src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java
index bbb99351a7f..8eb9056cd67 100644
--- a/src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java
+++ b/src/main/java/org/elasticsearch/index/mapper/ObjectMapperListener.java
@@ -21,10 +21,23 @@ package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.index.mapper.object.ObjectMapper;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  *
  */
 public interface ObjectMapperListener {
 
+    public static class Aggregator implements ObjectMapperListener {
+
+        public final List<ObjectMapper> objectMappers = new ArrayList<ObjectMapper>();
+
+        @Override
+        public void objectMapper(ObjectMapper objectMapper) {
+            objectMappers.add(objectMapper);
+        }
+    }
+
     void objectMapper(ObjectMapper objectMapper);
 }