Skip to content

Commit

Permalink
Fix a mappings update test (#33146)
Browse files Browse the repository at this point in the history
This commit fixes a mappings update test. The test is broken in the
sense that it passes, but for the wrong reason. The test here is testing
that if we make a mapping update but do not commit that mapping update
then the mapper service still maintains the previous document
mapper. This was not the case long, long ago when a mapping update would
update the in-memory state before the cluster state update was
committed. This test was passing, but it was passing because the mapping
update was never even updated. It was never even updated because it was
encountering a null pointer exception. Of course the in-memory state is
not going to be updated in that case, we are simply going to end up with
a failed cluster state update. Fixing that leads to another issue which
is that the mapping source does not even parse so again we would, of
course, end up with the in-memory state not being modified. We fix these
issues, assert that the result cluster state task completed
successfully, and finally that the in-memory state was not updated since
we never committed the resulting cluster state.
  • Loading branch information
jasontedor committed Aug 26, 2018
1 parent db75566 commit 73cc92d
Showing 1 changed file with 17 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,18 @@
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.cluster.metadata;

import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingClusterStateUpdateRequest;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.ClusterStateTaskExecutor;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexService;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.MapperParsingException;
Expand All @@ -37,6 +40,7 @@

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

public class MetaDataMappingServiceTests extends ESSingleNodeTestCase {

Expand Down Expand Up @@ -98,8 +102,18 @@ public void testMappingClusterStateUpdateDoesntChangeExistingIndices() throws Ex
final ClusterService clusterService = getInstanceFromNode(ClusterService.class);
// TODO - it will be nice to get a random mapping generator
final PutMappingClusterStateUpdateRequest request = new PutMappingClusterStateUpdateRequest().type("type");
request.source("{ \"properties\" { \"field\": { \"type\": \"text\" }}}");
mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request));
request.indices(new Index[] {indexService.index()});
request.source("{ \"properties\": { \"field\": { \"type\": \"text\" }}}");
final ClusterStateTaskExecutor.ClusterTasksResult<PutMappingClusterStateUpdateRequest> result =
mappingService.putMappingExecutor.execute(clusterService.state(), Collections.singletonList(request));
// the task completed successfully
assertThat(result.executionResults.size(), equalTo(1));
assertTrue(result.executionResults.values().iterator().next().isSuccess());
// the task really was a mapping update
assertThat(
indexService.mapperService().documentMapper("type").mappingSource(),
not(equalTo(result.resultingState.metaData().index("test").mapping("type").source())));
// since we never committed the cluster state update, the in-memory state is unchanged
assertThat(indexService.mapperService().documentMapper("type").mappingSource(), equalTo(currentMapping));
}

Expand All @@ -120,4 +134,5 @@ public void testClusterStateIsNotChangedWithIdenticalMappings() throws Exception

assertSame(result, result2);
}

}

0 comments on commit 73cc92d

Please sign in to comment.