Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping: Fix delete mapping race condition. #6531

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -428,10 +428,11 @@ public ClusterState execute(ClusterState currentState) {
String latestIndexWithout = null;
for (String indexName : request.indices()) {
IndexMetaData indexMetaData = currentState.metaData().index(indexName);
IndexMetaData.Builder indexBuilder = IndexMetaData.builder(indexMetaData);


if (indexMetaData != null) {
IndexMetaData.Builder indexBuilder = IndexMetaData.builder(indexMetaData);
boolean isLatestIndexWithout = true;

for (String type : request.types()) {
if (indexMetaData.mappings().containsKey(type)) {
indexBuilder.removeMapping(type);
Expand All @@ -443,12 +444,18 @@ public ClusterState execute(ClusterState currentState) {
latestIndexWithout = indexMetaData.index();
}

builder.put(indexBuilder);
}
builder.put(indexBuilder);
}

if (!changed) {
throw new TypeMissingException(new Index(latestIndexWithout), request.types());
if (latestIndexWithout == null){
//We never got a latestIndexWith since when we tried to access the index/mapping it
//had been deleted out from underneath us.
throw new TypeMissingException(new Index(request.indices()[0]), request.types());
} else {
throw new TypeMissingException(new Index(latestIndexWithout), request.types());
}
}

logger.info("[{}] remove_mapping [{}]", request.indices(), request.types());
Expand Down
@@ -0,0 +1,114 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.index.mapper.delete;

import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.action.admin.indices.create.CreateIndexResponse;
import org.elasticsearch.action.admin.indices.mapping.delete.DeleteMappingResponse;
import org.elasticsearch.indices.IndexAlreadyExistsException;
import org.elasticsearch.indices.IndexMissingException;
import org.elasticsearch.indices.TypeMissingException;
import org.elasticsearch.test.ElasticsearchIntegrationTest;
import org.junit.Test;

import java.util.ArrayList;
import java.util.List;

/**
*/
@ElasticsearchIntegrationTest.ClusterScope(scope = ElasticsearchIntegrationTest.Scope.SUITE)
public class MultithreadedDeleteMappingTest extends ElasticsearchIntegrationTest {

class CreateThread extends Thread{
public void run(){
for (int i=0; i<1000; ++i){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we could randomize the number of iterations and the number of threads used below?

try {
CreateIndexResponse cir = client().admin().indices().prepareCreate("test_index").setSource(" {\"mappings\":{" +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use assertAcked(prepareCreate(...)) here

" \"test_type\":{" +
" \"properties\":{" +
" \"text\":{" +
" \"type\": \"string\"," +
" \"analyzer\": \"whitespace\"}" +
" }" +
" }" +
" }" +
"}").execute().actionGet();
assertTrue(cir.isAcknowledged());
} catch( IndexAlreadyExistsException e ){

}
}
}
}

class DeleteThread extends Thread{
public void run() {
for (int i=0; i<1000; ++i) {
try {
DeleteMappingResponse dmr = client().admin().indices().prepareDeleteMapping("test_index").setType("test_type").execute().actionGet();
assertTrue(dmr.isAcknowledged());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertAcked here too

client().admin().indices().prepareDelete("test_index").execute().actionGet();
} catch ( IndexMissingException e) {

} catch( TypeMissingException te ){

}
}
}
}

public void startAll(List<Thread> threads){
for(Thread t : threads){
t.start();
}
}

public void joinAll(List<Thread> threads) {
for (Thread t : threads) {
boolean joined = false;
while(!joined) {
try {
t.join();
joined = true;
} catch (InterruptedException ie) {
}
}
}
}

@Test
public void testMultiThreadedMappingTest(){
List<Thread> createList = new ArrayList<>();
List<Thread> deleteList = new ArrayList<>();
for (int i = 0; i<10; ++i) {
createList.add(new CreateThread());
}

for (int i =0; i<10; ++i){
deleteList.add(new DeleteThread());
}

startAll(createList);
startAll(deleteList);

joinAll(createList);
joinAll(deleteList);
}
}