From 5ac42f2a4f098c66bfc1452960b4b5f3d90c7eac Mon Sep 17 00:00:00 2001
From: kimchy <kimchy@gmail.com>
Date: Thu, 23 Dec 2010 09:53:05 +0200
Subject: [PATCH] Excessive mapping parsing when cluster state changes, closes
 #575.

---
 .../index/mapper/xcontent/ObjectMapper.java          |  8 ++++++--
 .../indices/cluster/IndicesClusterStateService.java  | 12 ++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java
index d154a937a52..44da3130a10 100644
--- a/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java
+++ b/modules/elasticsearch/src/main/java/org/elasticsearch/index/mapper/xcontent/ObjectMapper.java
@@ -34,6 +34,7 @@ import java.io.IOException;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import static org.elasticsearch.common.collect.ImmutableMap.*;
 import static org.elasticsearch.common.collect.Lists.*;
@@ -575,8 +576,11 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper {
 
         doXContent(builder, params);
 
+        // sort the mappers so we get consistent serialization format
+        TreeMap<String, XContentMapper> sortedMappers = new TreeMap<String, XContentMapper>(mappers);
+
         // check internal mappers first (this is only relevant for root object)
-        for (XContentMapper mapper : mappers.values()) {
+        for (XContentMapper mapper : sortedMappers.values()) {
             if (mapper instanceof InternalMapper) {
                 mapper.toXContent(builder, params);
             }
@@ -589,7 +593,7 @@ public class ObjectMapper implements XContentMapper, IncludeInAllMapper {
 
         if (!mappers.isEmpty()) {
             builder.startObject("properties");
-            for (XContentMapper mapper : mappers.values()) {
+            for (XContentMapper mapper : sortedMappers.values()) {
                 if (!(mapper instanceof InternalMapper)) {
                     mapper.toXContent(builder, params);
                 }
diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java b/modules/elasticsearch/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java
index a71dcd33f18..d6c9e486c75 100644
--- a/modules/elasticsearch/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java
+++ b/modules/elasticsearch/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java
@@ -276,9 +276,9 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent<Indic
                     logger.debug("[{}] adding mapping [{}], source [{}]", index, mappingType, mappingSource.string());
                 }
                 mapperService.add(mappingType, mappingSource.string());
-//                if (!mapperService.documentMapper(mappingType).mappingSource().equals(mappingSource)) {
-//                    logger.warn("[{}] parsed mapping [{}], and got different sources\noriginal:\n{}\nparsed:\n{}", mappingType, mappingSource, mapperService.documentMapper(mappingType).mappingSource());
-//                }
+                if (!mapperService.documentMapper(mappingType).mappingSource().equals(mappingSource)) {
+                    logger.warn("[{}] parsed mapping [{}], and got different sources\noriginal:\n{}\nparsed:\n{}", index, mappingType, mappingSource, mapperService.documentMapper(mappingType).mappingSource());
+                }
                 nodeMappingCreatedAction.nodeMappingCreated(new NodeMappingCreatedAction.NodeMappingCreatedResponse(index, mappingType, event.state().nodes().localNodeId()));
             } else {
                 DocumentMapper existingMapper = mapperService.documentMapper(mappingType);
@@ -288,9 +288,9 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent<Indic
                         logger.debug("[{}] updating mapping [{}], source [{}]", index, mappingType, mappingSource.string());
                     }
                     mapperService.add(mappingType, mappingSource.string());
-//                    if (!mapperService.documentMapper(mappingType).mappingSource().equals(mappingSource)) {
-//                        logger.warn("[{}] parsed mapping [{}], and got different sources\noriginal:\n{}\nparsed:\n{}", mappingType, mappingSource, mapperService.documentMapper(mappingType).mappingSource());
-//                    }
+                    if (!mapperService.documentMapper(mappingType).mappingSource().equals(mappingSource)) {
+                        logger.warn("[{}] parsed mapping [{}], and got different sources\noriginal:\n{}\nparsed:\n{}", index, mappingType, mappingSource, mapperService.documentMapper(mappingType).mappingSource());
+                    }
                     nodeMappingCreatedAction.nodeMappingCreated(new NodeMappingCreatedAction.NodeMappingCreatedResponse(index, mappingType, event.state().nodes().localNodeId()));
                 }
             }