From 5d1f3d82d5d4ffeca9d85ab6a6aa2058a71df015 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 08:34:23 +0300 Subject: [PATCH 1/7] unit test for MataDataCreateIndexService.IndexCreationTask --- .../metadata/MetaDataCreateIndexService.java | 666 +++++++++--------- 1 file changed, 348 insertions(+), 318 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index 07a97b4f320cb..ab679f312c59f 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -21,6 +21,7 @@ import com.carrotsearch.hppc.cursors.ObjectCursor; import com.carrotsearch.hppc.cursors.ObjectObjectCursor; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.apache.lucene.util.CollectionUtil; @@ -93,7 +94,6 @@ import java.util.function.Predicate; import java.util.stream.IntStream; -import static org.elasticsearch.action.support.ContextPreservingActionListener.wrapPreservingContext; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_CREATION_DATE; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_INDEX_UUID; @@ -223,324 +223,14 @@ private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, request.settings(updatedSettingsBuilder.build()); clusterService.submitStateUpdateTask("create-index [" + request.index() + "], cause [" + request.cause() + "]", - new AckedClusterStateUpdateTask(Priority.URGENT, request, - wrapPreservingContext(listener, threadPool.getThreadContext())) { - - @Override - protected ClusterStateUpdateResponse newResponse(boolean acknowledged) { - return new ClusterStateUpdateResponse(acknowledged); - } - - @Override - public ClusterState execute(ClusterState currentState) throws Exception { - Index createdIndex = null; - String removalExtraInfo = null; - IndexRemovalReason removalReason = IndexRemovalReason.FAILURE; - try { - validate(request, currentState); - - for (Alias alias : request.aliases()) { - aliasValidator.validateAlias(alias, request.index(), currentState.metaData()); - } - - // we only find a template when its an API call (a new index) - // find templates, highest order are better matching - List templates = findTemplates(request, currentState); - - Map customs = new HashMap<>(); - - // add the request mapping - Map> mappings = new HashMap<>(); - - Map templatesAliases = new HashMap<>(); - - List templateNames = new ArrayList<>(); - - for (Map.Entry entry : request.mappings().entrySet()) { - mappings.put(entry.getKey(), MapperService.parseMapping(xContentRegistry, entry.getValue())); - } - - for (Map.Entry entry : request.customs().entrySet()) { - customs.put(entry.getKey(), entry.getValue()); - } - - final Index shrinkFromIndex = request.shrinkFrom(); - - if (shrinkFromIndex == null) { - // apply templates, merging the mappings into the request mapping if exists - for (IndexTemplateMetaData template : templates) { - templateNames.add(template.getName()); - for (ObjectObjectCursor cursor : template.mappings()) { - String mappingString = cursor.value.string(); - if (mappings.containsKey(cursor.key)) { - XContentHelper.mergeDefaults(mappings.get(cursor.key), - MapperService.parseMapping(xContentRegistry, mappingString)); - } else { - mappings.put(cursor.key, - MapperService.parseMapping(xContentRegistry, mappingString)); - } - } - // handle custom - for (ObjectObjectCursor cursor : template.customs()) { - String type = cursor.key; - IndexMetaData.Custom custom = cursor.value; - IndexMetaData.Custom existing = customs.get(type); - if (existing == null) { - customs.put(type, custom); - } else { - IndexMetaData.Custom merged = existing.mergeWith(custom); - customs.put(type, merged); - } - } - //handle aliases - for (ObjectObjectCursor cursor : template.aliases()) { - AliasMetaData aliasMetaData = cursor.value; - //if an alias with same name came with the create index request itself, - // ignore this one taken from the index template - if (request.aliases().contains(new Alias(aliasMetaData.alias()))) { - continue; - } - //if an alias with same name was already processed, ignore this one - if (templatesAliases.containsKey(cursor.key)) { - continue; - } - - //Allow templatesAliases to be templated by replacing a token with the name of the index that we are applying it to - if (aliasMetaData.alias().contains("{index}")) { - String templatedAlias = aliasMetaData.alias().replace("{index}", request.index()); - aliasMetaData = AliasMetaData.newAliasMetaData(aliasMetaData, templatedAlias); - } - - aliasValidator.validateAliasMetaData(aliasMetaData, request.index(), currentState.metaData()); - templatesAliases.put(aliasMetaData.alias(), aliasMetaData); - } - } - } - Settings.Builder indexSettingsBuilder = Settings.builder(); - if (shrinkFromIndex == null) { - // apply templates, here, in reverse order, since first ones are better matching - for (int i = templates.size() - 1; i >= 0; i--) { - indexSettingsBuilder.put(templates.get(i).settings()); - } - } - // now, put the request settings, so they override templates - indexSettingsBuilder.put(request.settings()); - if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) { - indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, 5)); - } - if (indexSettingsBuilder.get(SETTING_NUMBER_OF_REPLICAS) == null) { - indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, 1)); - } - if (settings.get(SETTING_AUTO_EXPAND_REPLICAS) != null && indexSettingsBuilder.get(SETTING_AUTO_EXPAND_REPLICAS) == null) { - indexSettingsBuilder.put(SETTING_AUTO_EXPAND_REPLICAS, settings.get(SETTING_AUTO_EXPAND_REPLICAS)); - } - - if (indexSettingsBuilder.get(SETTING_VERSION_CREATED) == null) { - DiscoveryNodes nodes = currentState.nodes(); - final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion()); - indexSettingsBuilder.put(SETTING_VERSION_CREATED, createdVersion); - } - - if (indexSettingsBuilder.get(SETTING_CREATION_DATE) == null) { - indexSettingsBuilder.put(SETTING_CREATION_DATE, new DateTime(DateTimeZone.UTC).getMillis()); - } - indexSettingsBuilder.put(IndexMetaData.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName()); - indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID()); - final IndexMetaData.Builder tmpImdBuilder = IndexMetaData.builder(request.index()); - - final int routingNumShards; - if (shrinkFromIndex == null) { - routingNumShards = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(indexSettingsBuilder.build()); - } else { - final IndexMetaData sourceMetaData = currentState.metaData().getIndexSafe(shrinkFromIndex); - routingNumShards = sourceMetaData.getRoutingNumShards(); - } - tmpImdBuilder.setRoutingNumShards(routingNumShards); - - if (shrinkFromIndex != null) { - prepareShrinkIndexSettings( - currentState, mappings.keySet(), indexSettingsBuilder, shrinkFromIndex, request.index()); - } - final Settings actualIndexSettings = indexSettingsBuilder.build(); - tmpImdBuilder.settings(actualIndexSettings); - - if (shrinkFromIndex != null) { - /* - * We need to arrange that the primary term on all the shards in the shrunken index is at least as large as - * the maximum primary term on all the shards in the source index. This ensures that we have correct - * document-level semantics regarding sequence numbers in the shrunken index. - */ - final IndexMetaData sourceMetaData = currentState.metaData().getIndexSafe(shrinkFromIndex); - final long primaryTerm = - IntStream - .range(0, sourceMetaData.getNumberOfShards()) - .mapToLong(sourceMetaData::primaryTerm) - .max() - .getAsLong(); - for (int shardId = 0; shardId < tmpImdBuilder.numberOfShards(); shardId++) { - tmpImdBuilder.primaryTerm(shardId, primaryTerm); - } - } - - // Set up everything, now locally create the index to see that things are ok, and apply - final IndexMetaData tmpImd = tmpImdBuilder.build(); - ActiveShardCount waitForActiveShards = request.waitForActiveShards(); - if (waitForActiveShards == ActiveShardCount.DEFAULT) { - waitForActiveShards = tmpImd.getWaitForActiveShards(); - } - if (waitForActiveShards.validate(tmpImd.getNumberOfReplicas()) == false) { - throw new IllegalArgumentException("invalid wait_for_active_shards[" + request.waitForActiveShards() + - "]: cannot be greater than number of shard copies [" + - (tmpImd.getNumberOfReplicas() + 1) + "]"); - } - // create the index here (on the master) to validate it can be created, as well as adding the mapping - final IndexService indexService = indicesService.createIndex(tmpImd, Collections.emptyList()); - createdIndex = indexService.index(); - // now add the mappings - MapperService mapperService = indexService.mapperService(); - try { - mapperService.merge(mappings, MergeReason.MAPPING_UPDATE, request.updateAllTypes()); - } catch (Exception e) { - removalExtraInfo = "failed on parsing default mapping/mappings on index creation"; - throw e; - } - - if (request.shrinkFrom() == null) { - // now that the mapping is merged we can validate the index sort. - // we cannot validate for index shrinking since the mapping is empty - // at this point. The validation will take place later in the process - // (when all shards are copied in a single place). - indexService.getIndexSortSupplier().get(); - } - - // the context is only used for validation so it's fine to pass fake values for the shard id and the current - // timestamp - final QueryShardContext queryShardContext = indexService.newQueryShardContext(0, null, () -> 0L, null); - - for (Alias alias : request.aliases()) { - if (Strings.hasLength(alias.filter())) { - aliasValidator.validateAliasFilter(alias.name(), alias.filter(), queryShardContext, xContentRegistry); - } - } - for (AliasMetaData aliasMetaData : templatesAliases.values()) { - if (aliasMetaData.filter() != null) { - aliasValidator.validateAliasFilter(aliasMetaData.alias(), aliasMetaData.filter().uncompressed(), - queryShardContext, xContentRegistry); - } - } - - // now, update the mappings with the actual source - Map mappingsMetaData = new HashMap<>(); - for (DocumentMapper mapper : mapperService.docMappers(true)) { - MappingMetaData mappingMd = new MappingMetaData(mapper); - mappingsMetaData.put(mapper.type(), mappingMd); - } - - final IndexMetaData.Builder indexMetaDataBuilder = IndexMetaData.builder(request.index()) - .settings(actualIndexSettings) - .setRoutingNumShards(routingNumShards); - - for (int shardId = 0; shardId < tmpImd.getNumberOfShards(); shardId++) { - indexMetaDataBuilder.primaryTerm(shardId, tmpImd.primaryTerm(shardId)); - } - - for (MappingMetaData mappingMd : mappingsMetaData.values()) { - indexMetaDataBuilder.putMapping(mappingMd); - } - - for (AliasMetaData aliasMetaData : templatesAliases.values()) { - indexMetaDataBuilder.putAlias(aliasMetaData); - } - for (Alias alias : request.aliases()) { - AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter()) - .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build(); - indexMetaDataBuilder.putAlias(aliasMetaData); - } - - for (Map.Entry customEntry : customs.entrySet()) { - indexMetaDataBuilder.putCustom(customEntry.getKey(), customEntry.getValue()); - } - - indexMetaDataBuilder.state(request.state()); - - final IndexMetaData indexMetaData; - try { - indexMetaData = indexMetaDataBuilder.build(); - } catch (Exception e) { - removalExtraInfo = "failed to build index metadata"; - throw e; - } - - indexService.getIndexEventListener().beforeIndexAddedToCluster(indexMetaData.getIndex(), - indexMetaData.getSettings()); - - MetaData newMetaData = MetaData.builder(currentState.metaData()) - .put(indexMetaData, false) - .build(); - - logger.info("[{}] creating index, cause [{}], templates {}, shards [{}]/[{}], mappings {}", - request.index(), request.cause(), templateNames, indexMetaData.getNumberOfShards(), - indexMetaData.getNumberOfReplicas(), mappings.keySet()); - - ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); - if (!request.blocks().isEmpty()) { - for (ClusterBlock block : request.blocks()) { - blocks.addIndexBlock(request.index(), block); - } - } - blocks.updateBlocks(indexMetaData); - - ClusterState updatedState = ClusterState.builder(currentState).blocks(blocks).metaData(newMetaData).build(); - - if (request.state() == State.OPEN) { - RoutingTable.Builder routingTableBuilder = RoutingTable.builder(updatedState.routingTable()) - .addAsNew(updatedState.metaData().index(request.index())); - updatedState = allocationService.reroute( - ClusterState.builder(updatedState).routingTable(routingTableBuilder.build()).build(), - "index [" + request.index() + "] created"); - } - removalExtraInfo = "cleaning up after validating index on master"; - removalReason = IndexRemovalReason.NO_LONGER_ASSIGNED; - return updatedState; - } finally { - if (createdIndex != null) { - // Index was already partially created - need to clean up - indicesService.removeIndex(createdIndex, removalReason, removalExtraInfo); - } - } - } - - @Override - public void onFailure(String source, Exception e) { - if (e instanceof ResourceAlreadyExistsException) { - logger.trace((Supplier) () -> new ParameterizedMessage("[{}] failed to create", request.index()), e); - } else { - logger.debug((Supplier) () -> new ParameterizedMessage("[{}] failed to create", request.index()), e); - } - super.onFailure(source, e); - } - }); - } - - private List findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state) throws IOException { - List templateMetadata = new ArrayList<>(); - for (ObjectCursor cursor : state.metaData().templates().values()) { - IndexTemplateMetaData metadata = cursor.value; - for (String template: metadata.patterns()) { - if (Regex.simpleMatch(template, request.index())) { - templateMetadata.add(metadata); - break; + new IndexCreationTask(logger, allocationService, request, listener, + indicesService, aliasValidator, xContentRegistry, settings, + (req, state) -> { + validateIndexName(req.index(), state); + validateIndexSettings(req.index(), req.settings()); } - } - } - - CollectionUtil.timSort(templateMetadata, Comparator.comparingInt(IndexTemplateMetaData::order).reversed()); - return templateMetadata; - } - - private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) { - validateIndexName(request.index(), state); - validateIndexSettings(request.index(), request.settings()); + ) + ); } public void validateIndexSettings(String indexName, Settings settings) throws IndexCreationException { @@ -648,4 +338,344 @@ static void prepareShrinkIndexSettings(ClusterState currentState, Set ma .put(IndexMetaData.INDEX_SHRINK_SOURCE_UUID.getKey(), shrinkFromIndex.getUUID()); } + interface IndexValidator { + void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state); + } + + static class IndexCreationTask extends AckedClusterStateUpdateTask { + + private final IndicesService indicesService; + private final AliasValidator aliasValidator; + private final NamedXContentRegistry xContentRegistry; + private final CreateIndexClusterStateUpdateRequest request; + private final Logger logger; + private final AllocationService allocationService; + private final Settings settings; + private final IndexValidator validator; + + IndexCreationTask(Logger logger, AllocationService allocationService, CreateIndexClusterStateUpdateRequest request, + ActionListener listener, IndicesService indicesService, + AliasValidator aliasValidator, NamedXContentRegistry xContentRegistry, + Settings settings, IndexValidator validator) { + super(Priority.URGENT, request, listener); + this.request = request; + this.logger = logger; + this.allocationService = allocationService; + this.indicesService = indicesService; + this.aliasValidator = aliasValidator; + this.xContentRegistry = xContentRegistry; + this.settings = settings; + this.validator = validator; + } + + @Override + protected ClusterStateUpdateResponse newResponse(boolean acknowledged) { + return new ClusterStateUpdateResponse(acknowledged); + } + + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + Index createdIndex = null; + String removalExtraInfo = null; + IndexRemovalReason removalReason = IndexRemovalReason.FAILURE; + try { + validator.validate(request, currentState); + + for (Alias alias : request.aliases()) { + aliasValidator.validateAlias(alias, request.index(), currentState.metaData()); + } + + // we only find a template when its an API call (a new index) + // find templates, highest order are better matching + List templates = findTemplates(request, currentState); + + Map customs = new HashMap<>(); + + // add the request mapping + Map> mappings = new HashMap<>(); + + Map templatesAliases = new HashMap<>(); + + List templateNames = new ArrayList<>(); + + for (Map.Entry entry : request.mappings().entrySet()) { + mappings.put(entry.getKey(), MapperService.parseMapping(xContentRegistry, entry.getValue())); + } + + for (Map.Entry entry : request.customs().entrySet()) { + customs.put(entry.getKey(), entry.getValue()); + } + + final Index shrinkFromIndex = request.shrinkFrom(); + + if (shrinkFromIndex == null) { + // apply templates, merging the mappings into the request mapping if exists + for (IndexTemplateMetaData template : templates) { + templateNames.add(template.getName()); + for (ObjectObjectCursor cursor : template.mappings()) { + String mappingString = cursor.value.string(); + if (mappings.containsKey(cursor.key)) { + XContentHelper.mergeDefaults(mappings.get(cursor.key), + MapperService.parseMapping(xContentRegistry, mappingString)); + } else { + mappings.put(cursor.key, + MapperService.parseMapping(xContentRegistry, mappingString)); + } + } + // handle custom + for (ObjectObjectCursor cursor : template.customs()) { + String type = cursor.key; + Custom custom = cursor.value; + Custom existing = customs.get(type); + if (existing == null) { + customs.put(type, custom); + } else { + Custom merged = existing.mergeWith(custom); + customs.put(type, merged); + } + } + //handle aliases + for (ObjectObjectCursor cursor : template.aliases()) { + AliasMetaData aliasMetaData = cursor.value; + //if an alias with same name came with the create index request itself, + // ignore this one taken from the index template + if (request.aliases().contains(new Alias(aliasMetaData.alias()))) { + continue; + } + //if an alias with same name was already processed, ignore this one + if (templatesAliases.containsKey(cursor.key)) { + continue; + } + + //Allow templatesAliases to be templated by replacing a token with the name of the index that we are applying it to + if (aliasMetaData.alias().contains("{index}")) { + String templatedAlias = aliasMetaData.alias().replace("{index}", request.index()); + aliasMetaData = AliasMetaData.newAliasMetaData(aliasMetaData, templatedAlias); + } + + aliasValidator.validateAliasMetaData(aliasMetaData, request.index(), currentState.metaData()); + templatesAliases.put(aliasMetaData.alias(), aliasMetaData); + } + } + } + Settings.Builder indexSettingsBuilder = Settings.builder(); + if (shrinkFromIndex == null) { + // apply templates, here, in reverse order, since first ones are better matching + for (int i = templates.size() - 1; i >= 0; i--) { + indexSettingsBuilder.put(templates.get(i).settings()); + } + } + // now, put the request settings, so they override templates + indexSettingsBuilder.put(request.settings()); + if (indexSettingsBuilder.get(SETTING_NUMBER_OF_SHARDS) == null) { + indexSettingsBuilder.put(SETTING_NUMBER_OF_SHARDS, settings.getAsInt(SETTING_NUMBER_OF_SHARDS, 5)); + } + if (indexSettingsBuilder.get(SETTING_NUMBER_OF_REPLICAS) == null) { + indexSettingsBuilder.put(SETTING_NUMBER_OF_REPLICAS, settings.getAsInt(SETTING_NUMBER_OF_REPLICAS, 1)); + } + if (settings.get(SETTING_AUTO_EXPAND_REPLICAS) != null && indexSettingsBuilder.get(SETTING_AUTO_EXPAND_REPLICAS) == null) { + indexSettingsBuilder.put(SETTING_AUTO_EXPAND_REPLICAS, settings.get(SETTING_AUTO_EXPAND_REPLICAS)); + } + + if (indexSettingsBuilder.get(SETTING_VERSION_CREATED) == null) { + DiscoveryNodes nodes = currentState.nodes(); + final Version createdVersion = Version.min(Version.CURRENT, nodes.getSmallestNonClientNodeVersion()); + indexSettingsBuilder.put(SETTING_VERSION_CREATED, createdVersion); + } + + if (indexSettingsBuilder.get(SETTING_CREATION_DATE) == null) { + indexSettingsBuilder.put(SETTING_CREATION_DATE, new DateTime(DateTimeZone.UTC).getMillis()); + } + indexSettingsBuilder.put(IndexMetaData.SETTING_INDEX_PROVIDED_NAME, request.getProvidedName()); + indexSettingsBuilder.put(SETTING_INDEX_UUID, UUIDs.randomBase64UUID()); + final IndexMetaData.Builder tmpImdBuilder = IndexMetaData.builder(request.index()); + + final int routingNumShards; + if (shrinkFromIndex == null) { + routingNumShards = IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(indexSettingsBuilder.build()); + } else { + final IndexMetaData sourceMetaData = currentState.metaData().getIndexSafe(shrinkFromIndex); + routingNumShards = sourceMetaData.getRoutingNumShards(); + } + tmpImdBuilder.setRoutingNumShards(routingNumShards); + + if (shrinkFromIndex != null) { + prepareShrinkIndexSettings( + currentState, mappings.keySet(), indexSettingsBuilder, shrinkFromIndex, request.index()); + } + final Settings actualIndexSettings = indexSettingsBuilder.build(); + tmpImdBuilder.settings(actualIndexSettings); + + if (shrinkFromIndex != null) { + /* + * We need to arrange that the primary term on all the shards in the shrunken index is at least as large as + * the maximum primary term on all the shards in the source index. This ensures that we have correct + * document-level semantics regarding sequence numbers in the shrunken index. + */ + final IndexMetaData sourceMetaData = currentState.metaData().getIndexSafe(shrinkFromIndex); + final long primaryTerm = + IntStream + .range(0, sourceMetaData.getNumberOfShards()) + .mapToLong(sourceMetaData::primaryTerm) + .max() + .getAsLong(); + for (int shardId = 0; shardId < tmpImdBuilder.numberOfShards(); shardId++) { + tmpImdBuilder.primaryTerm(shardId, primaryTerm); + } + } + + // Set up everything, now locally create the index to see that things are ok, and apply + final IndexMetaData tmpImd = tmpImdBuilder.build(); + ActiveShardCount waitForActiveShards = request.waitForActiveShards(); + if (waitForActiveShards == ActiveShardCount.DEFAULT) { + waitForActiveShards = tmpImd.getWaitForActiveShards(); + } + if (waitForActiveShards.validate(tmpImd.getNumberOfReplicas()) == false) { + throw new IllegalArgumentException("invalid wait_for_active_shards[" + request.waitForActiveShards() + + "]: cannot be greater than number of shard copies [" + + (tmpImd.getNumberOfReplicas() + 1) + "]"); + } + // create the index here (on the master) to validate it can be created, as well as adding the mapping + final IndexService indexService = indicesService.createIndex(tmpImd, Collections.emptyList()); + createdIndex = indexService.index(); + // now add the mappings + MapperService mapperService = indexService.mapperService(); + try { + mapperService.merge(mappings, MergeReason.MAPPING_UPDATE, request.updateAllTypes()); + } catch (Exception e) { + removalExtraInfo = "failed on parsing default mapping/mappings on index creation"; + throw e; + } + + if (request.shrinkFrom() == null) { + // now that the mapping is merged we can validate the index sort. + // we cannot validate for index shrinking since the mapping is empty + // at this point. The validation will take place later in the process + // (when all shards are copied in a single place). + indexService.getIndexSortSupplier().get(); + } + + // the context is only used for validation so it's fine to pass fake values for the shard id and the current + // timestamp + final QueryShardContext queryShardContext = indexService.newQueryShardContext(0, null, () -> 0L, null); + + for (Alias alias : request.aliases()) { + if (Strings.hasLength(alias.filter())) { + aliasValidator.validateAliasFilter(alias.name(), alias.filter(), queryShardContext, xContentRegistry); + } + } + for (AliasMetaData aliasMetaData : templatesAliases.values()) { + if (aliasMetaData.filter() != null) { + aliasValidator.validateAliasFilter(aliasMetaData.alias(), aliasMetaData.filter().uncompressed(), + queryShardContext, xContentRegistry); + } + } + + // now, update the mappings with the actual source + Map mappingsMetaData = new HashMap<>(); + for (DocumentMapper mapper : mapperService.docMappers(true)) { + MappingMetaData mappingMd = new MappingMetaData(mapper); + mappingsMetaData.put(mapper.type(), mappingMd); + } + + final IndexMetaData.Builder indexMetaDataBuilder = IndexMetaData.builder(request.index()) + .settings(actualIndexSettings) + .setRoutingNumShards(routingNumShards); + + for (int shardId = 0; shardId < tmpImd.getNumberOfShards(); shardId++) { + indexMetaDataBuilder.primaryTerm(shardId, tmpImd.primaryTerm(shardId)); + } + + for (MappingMetaData mappingMd : mappingsMetaData.values()) { + indexMetaDataBuilder.putMapping(mappingMd); + } + + for (AliasMetaData aliasMetaData : templatesAliases.values()) { + indexMetaDataBuilder.putAlias(aliasMetaData); + } + for (Alias alias : request.aliases()) { + AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter()) + .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build(); + indexMetaDataBuilder.putAlias(aliasMetaData); + } + + for (Map.Entry customEntry : customs.entrySet()) { + indexMetaDataBuilder.putCustom(customEntry.getKey(), customEntry.getValue()); + } + + indexMetaDataBuilder.state(request.state()); + + final IndexMetaData indexMetaData; + try { + indexMetaData = indexMetaDataBuilder.build(); + } catch (Exception e) { + removalExtraInfo = "failed to build index metadata"; + throw e; + } + + indexService.getIndexEventListener().beforeIndexAddedToCluster(indexMetaData.getIndex(), + indexMetaData.getSettings()); + + MetaData newMetaData = MetaData.builder(currentState.metaData()) + .put(indexMetaData, false) + .build(); + + logger.info("[{}] creating index, cause [{}], templates {}, shards [{}]/[{}], mappings {}", + request.index(), request.cause(), templateNames, indexMetaData.getNumberOfShards(), + indexMetaData.getNumberOfReplicas(), mappings.keySet()); + + ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); + if (!request.blocks().isEmpty()) { + for (ClusterBlock block : request.blocks()) { + blocks.addIndexBlock(request.index(), block); + } + } + blocks.updateBlocks(indexMetaData); + + ClusterState updatedState = ClusterState.builder(currentState).blocks(blocks).metaData(newMetaData).build(); + + if (request.state() == State.OPEN) { + RoutingTable.Builder routingTableBuilder = RoutingTable.builder(updatedState.routingTable()) + .addAsNew(updatedState.metaData().index(request.index())); + updatedState = allocationService.reroute( + ClusterState.builder(updatedState).routingTable(routingTableBuilder.build()).build(), + "index [" + request.index() + "] created"); + } + removalExtraInfo = "cleaning up after validating index on master"; + removalReason = IndexRemovalReason.NO_LONGER_ASSIGNED; + return updatedState; + } finally { + if (createdIndex != null) { + // Index was already partially created - need to clean up + indicesService.removeIndex(createdIndex, removalReason, removalExtraInfo); + } + } + } + + @Override + public void onFailure(String source, Exception e) { + if (e instanceof ResourceAlreadyExistsException) { + logger.trace((Supplier) () -> new ParameterizedMessage("[{}] failed to create", request.index()), e); + } else { + logger.debug((Supplier) () -> new ParameterizedMessage("[{}] failed to create", request.index()), e); + } + super.onFailure(source, e); + } + + private List findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state) throws IOException { + List templateMetadata = new ArrayList<>(); + for (ObjectCursor cursor : state.metaData().templates().values()) { + IndexTemplateMetaData metadata = cursor.value; + for (String template: metadata.patterns()) { + if (Regex.simpleMatch(template, request.index())) { + templateMetadata.add(metadata); + break; + } + } + } + + CollectionUtil.timSort(templateMetadata, Comparator.comparingInt(IndexTemplateMetaData::order).reversed()); + return templateMetadata; + } + } } From 0054af0e749510034df57027763f0503d6d74ff9 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 08:34:40 +0300 Subject: [PATCH 2/7] unit test for MataDataCreateIndexService.IndexCreationTask --- .../metadata/IndexCreationTaskTests.java | 450 ++++++++++++++++++ 1 file changed, 450 insertions(+) create mode 100644 core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java new file mode 100644 index 0000000000000..8f2b1d6668c8f --- /dev/null +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java @@ -0,0 +1,450 @@ +/* + * 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.cluster.metadata; + +import org.apache.logging.log4j.Logger; +import org.apache.lucene.search.Sort; +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.indices.alias.Alias; +import org.elasticsearch.action.admin.indices.create.CreateIndexClusterStateUpdateRequest; +import org.elasticsearch.action.support.ActiveShardCount; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.block.ClusterBlock; +import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.block.ClusterBlocks; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.routing.IndexRoutingTable; +import org.elasticsearch.cluster.routing.RoutingTable; +import org.elasticsearch.cluster.routing.ShardRoutingState; +import org.elasticsearch.cluster.routing.TestShardRouting; +import org.elasticsearch.cluster.routing.allocation.AllocationService; +import org.elasticsearch.common.collect.ImmutableOpenMap; +import org.elasticsearch.common.compress.CompressedXContent; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.index.Index; +import org.elasticsearch.index.IndexService; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.index.mapper.ParentFieldMapper; +import org.elasticsearch.index.mapper.RoutingFieldMapper; +import org.elasticsearch.index.shard.IndexEventListener; +import org.elasticsearch.indices.IndicesService; +import org.elasticsearch.test.ESTestCase; +import org.mockito.ArgumentCaptor; + +import java.io.IOException; +import java.util.Map; +import java.util.HashSet; +import java.util.Set; +import java.util.Collections; +import java.util.Arrays; +import java.util.function.Supplier; + +import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyObject; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.anyMap; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.eq; + +public class IndexCreationTaskTests extends ESTestCase { + + private final IndicesService indicesService = mock(IndicesService.class); + private final AliasValidator aliasValidator = mock(AliasValidator.class); + private final NamedXContentRegistry xContentRegistry = mock(NamedXContentRegistry.class); + private final CreateIndexClusterStateUpdateRequest request = mock(CreateIndexClusterStateUpdateRequest.class); + private final Logger logger = mock(Logger.class); + private final AllocationService allocationService = mock(AllocationService.class); + private final MetaDataCreateIndexService.IndexValidator validator = mock(MetaDataCreateIndexService.IndexValidator.class); + private final ActionListener listener = mock(ActionListener.class); + private final ClusterState state = mock(ClusterState.class); + private final Settings.Builder clusterStateSettings = Settings.builder(); + private final MapperService mapper = mock(MapperService.class); + + private final ImmutableOpenMap.Builder tplBuilder = ImmutableOpenMap.builder(); + private final ImmutableOpenMap.Builder customBuilder = ImmutableOpenMap.builder(); + private final ImmutableOpenMap.Builder idxBuilder = ImmutableOpenMap.builder(); + + private final Settings.Builder reqSettings = Settings.builder(); + private final Set reqBlocks = Sets.newHashSet(); + private final MetaData.Builder currentStateMetaDataBuilder = MetaData.builder(); + private final ClusterBlocks currentStateBlocks = mock(ClusterBlocks.class); + private final RoutingTable.Builder routingTableBuilder = RoutingTable.builder(); + private final DocumentMapper docMapper = mock(DocumentMapper.class); + + private ActiveShardCount waitForActiveShardsNum = ActiveShardCount.DEFAULT; + + public void setUp() throws Exception { + super.setUp(); + setupIndicesService(); + setupClusterState(); + } + + public void testMatchTemplates() throws Exception { + tplBuilder.put("template_1", createTemplateMetadata("template_1", "te*")); + tplBuilder.put("template_2", createTemplateMetadata("template_2", "tes*")); + tplBuilder.put("template_3", createTemplateMetadata("template_3", "zzz*")); + + final ClusterState result = executeTask(); + + assertTrue(result.metaData().index("test").getAliases().containsKey("alias_from_template_1")); + assertTrue(result.metaData().index("test").getAliases().containsKey("alias_from_template_2")); + assertFalse(result.metaData().index("test").getAliases().containsKey("alias_from_template_3")); + } + + public void testApplyDataFromTemplate() throws Exception { + addMatchingTemplate(builder -> builder + .putAlias(AliasMetaData.builder("alias1")) + .putMapping("mapping1", createMapping()) + .putCustom("custom1", createCustom()) + .settings(Settings.builder().put("key1", "value1")) + ); + + final ClusterState result = executeTask(); + + assertTrue(result.metaData().index("test").getAliases().containsKey("alias1")); + assertTrue(result.metaData().index("test").getCustoms().containsKey("custom1")); + assertEquals("value1", result.metaData().index("test").getSettings().get("key1")); + assertTrue(getMappingsFromResponse().containsKey("mapping1")); + } + + public void testApplyDataFromRequest() throws Exception { + setupRequestAlias(new Alias("alias1")); + setupRequestMapping("mapping1", createMapping()); + setupRequestCustom("custom1", createCustom()); + reqSettings.put("key1", "value1"); + + final ClusterState result = executeTask(); + + assertTrue(result.metaData().index("test").getAliases().containsKey("alias1")); + assertTrue(result.metaData().index("test").getCustoms().containsKey("custom1")); + assertEquals("value1", result.metaData().index("test").getSettings().get("key1")); + assertTrue(getMappingsFromResponse().containsKey("mapping1")); + } + + public void testRequestDataHavePriorityOverTemplateData() throws Exception { + final IndexMetaData.Custom tplCustom = createCustom(); + final IndexMetaData.Custom reqCustom = createCustom(); + final IndexMetaData.Custom mergedCustom = createCustom(); + when(reqCustom.mergeWith(tplCustom)).thenReturn(mergedCustom); + + final CompressedXContent tplMapping = createMapping("text"); + final CompressedXContent reqMapping = createMapping("keyword"); + + addMatchingTemplate(builder -> builder + .putAlias(AliasMetaData.builder("alias1").searchRouting("fromTpl").build()) + .putMapping("mapping1", tplMapping) + .putCustom("custom1", tplCustom) + .settings(Settings.builder().put("key1", "tplValue")) + ); + + setupRequestAlias(new Alias("alias1").searchRouting("fromReq")); + setupRequestMapping("mapping1", reqMapping); + setupRequestCustom("custom1", reqCustom); + reqSettings.put("key1", "reqValue"); + + final ClusterState result = executeTask(); + + assertEquals(mergedCustom, result.metaData().index("test").getCustoms().get("custom1")); + assertEquals("fromReq", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); + assertEquals("reqValue", result.metaData().index("test").getSettings().get("key1")); + assertEquals("{type={properties={field={type=keyword}}}}", getMappingsFromResponse().get("mapping1").toString()); + } + + public void testDefaultSettings() throws Exception { + final ClusterState result = executeTask(); + + assertEquals("5", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + } + + public void testSettingsFromClusterState() throws Exception { + clusterStateSettings.put(SETTING_NUMBER_OF_SHARDS, 15); + + final ClusterState result = executeTask(); + + assertEquals("15", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + } + + public void testTemplateOrder() throws Exception { + addMatchingTemplate(builder -> builder + .order(1) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 10)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("1").build()) + ); + addMatchingTemplate(builder -> builder + .order(2) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 11)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("2").build()) + ); + addMatchingTemplate(builder -> builder + .order(3) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 12)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("3").build()) + ); + final ClusterState result = executeTask(); + + assertEquals("12", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + assertEquals("3", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); + } + + public void testTemplateOrder2() throws Exception { + addMatchingTemplate(builder -> builder + .order(3) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 12)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("3").build()) + ); + addMatchingTemplate(builder -> builder + .order(2) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 11)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("2").build()) + ); + addMatchingTemplate(builder -> builder + .order(1) + .settings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 10)) + .putAlias(AliasMetaData.builder("alias1").searchRouting("1").build()) + ); + final ClusterState result = executeTask(); + + assertEquals("12", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + assertEquals("3", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); + } + + public void testRequestStateOpen() throws Exception { + + when(request.state()).thenReturn(IndexMetaData.State.OPEN); + + executeTask(); + + verify(allocationService, times(1)).reroute(anyObject(), anyObject()); + } + + @SuppressWarnings("unchecked") + public void testIndexRemovalOnFailure() throws Exception { + doThrow(new RuntimeException("oops")).when(mapper).merge(anyMap(), anyObject(), anyBoolean()); + + try { + executeTask(); + fail("exception not thrown"); + } catch (RuntimeException e) { + verify(indicesService, times(1)).removeIndex(anyObject(), anyObject(), anyObject()); + } + } + + public void testShrinkIndexIgnoresTemplates() throws Exception { + final Index source = new Index("source_idx", "aaa111bbb222"); + + when(request.shrinkFrom()).thenReturn(source); + + currentStateMetaDataBuilder.put(createIndexMetaDataBuilder("source_idx", "aaa111bbb222", 2, 2)); + + routingTableBuilder.add(createIndexRoutingTableWithStartedShards(source)); + + when(currentStateBlocks.indexBlocked(eq(ClusterBlockLevel.WRITE), eq("source_idx"))).thenReturn(true); + reqSettings.put(SETTING_NUMBER_OF_SHARDS, 1); + + addMatchingTemplate(builder -> builder + .putAlias(AliasMetaData.builder("alias1").searchRouting("fromTpl").build()) + .putMapping("mapping1", createMapping()) + .putCustom("custom1", createCustom()) + .settings(Settings.builder().put("key1", "tplValue")) + ); + + final ClusterState result = executeTask(); + + assertFalse(result.metaData().index("test").getAliases().containsKey("alias1")); + assertFalse(result.metaData().index("test").getCustoms().containsKey("custom1")); + assertNull(result.metaData().index("test").getSettings().get("key1")); + assertFalse(getMappingsFromResponse().containsKey("mapping1")); + } + + public void testValidateWaitForActiveShardsFailure() throws Exception { + waitForActiveShardsNum = ActiveShardCount.from(1000); + + try { + executeTask(); + fail("validation exception expected"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("invalid wait_for_active_shards")); + } + } + + private IndexRoutingTable createIndexRoutingTableWithStartedShards(Index index) { + final IndexRoutingTable idxRoutingTable = mock(IndexRoutingTable.class); + + when(idxRoutingTable.getIndex()).thenReturn(index); + when(idxRoutingTable.shardsWithState(eq(ShardRoutingState.STARTED))).thenReturn(Arrays.asList( + TestShardRouting.newShardRouting(index.getName(), 0, "1", randomBoolean(), ShardRoutingState.INITIALIZING).moveToStarted(), + TestShardRouting.newShardRouting(index.getName(), 0, "1", randomBoolean(), ShardRoutingState.INITIALIZING).moveToStarted() + + )); + + return idxRoutingTable; + } + + private IndexMetaData.Builder createIndexMetaDataBuilder(String name, String uuid, int numShards, int numReplicas) { + return IndexMetaData + .builder(name) + .settings(Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT) + .put(IndexMetaData.SETTING_INDEX_UUID, uuid)) + .putMapping(new MappingMetaData(docMapper)) + .numberOfShards(numShards) + .numberOfReplicas(numReplicas); + } + + private IndexMetaData.Custom createCustom() { + return mock(IndexMetaData.Custom.class); + } + + private interface MetaDataBuilderConfigurator { + void configure(IndexTemplateMetaData.Builder builder) throws IOException; + } + + private void addMatchingTemplate(MetaDataBuilderConfigurator configurator) throws IOException { + final IndexTemplateMetaData.Builder builder = metaDataBuilder("template1", "te*"); + configurator.configure(builder); + + tplBuilder.put("template" + builder.hashCode(), builder.build()); + } + + @SuppressWarnings("unchecked") + private Map> getMappingsFromResponse() { + final ArgumentCaptor argument = ArgumentCaptor.forClass(Map.class); + verify(mapper).merge(argument.capture(), anyObject(), anyBoolean()); + return argument.getValue(); + } + + private void setupRequestAlias(Alias alias) { + when(request.aliases()).thenReturn(new HashSet<>(Collections.singletonList(alias))); + } + + private void setupRequestMapping(String mappingKey, CompressedXContent mapping) throws IOException { + when(request.mappings()).thenReturn(Collections.singletonMap(mappingKey, mapping.string())); + } + + private void setupRequestCustom(String customKey, IndexMetaData.Custom custom) throws IOException { + when(request.customs()).thenReturn(Collections.singletonMap(customKey, custom)); + } + + private CompressedXContent createMapping() throws IOException { + return createMapping("text"); + } + + private CompressedXContent createMapping(String fieldType) throws IOException { + final String mapping = XContentFactory.jsonBuilder() + .startObject() + .startObject("type") + .startObject("properties") + .startObject("field") + .field("type", fieldType) + .endObject() + .endObject() + .endObject() + .endObject().string(); + + return new CompressedXContent(mapping); + } + + private IndexTemplateMetaData.Builder metaDataBuilder(String name, String pattern) { + return IndexTemplateMetaData + .builder(name) + .patterns(Collections.singletonList(pattern)); + } + + private IndexTemplateMetaData createTemplateMetadata(String name, String pattern) { + return IndexTemplateMetaData + .builder(name) + .patterns(Collections.singletonList(pattern)) + .putAlias(AliasMetaData.builder("alias_from_" + name).build()) + .build(); + } + + @SuppressWarnings("unchecked") + private ClusterState executeTask() throws Exception { + setupState(); + setupRequest(); + final MetaDataCreateIndexService.IndexCreationTask task = new MetaDataCreateIndexService.IndexCreationTask( + logger, allocationService, request, listener, indicesService, aliasValidator, xContentRegistry, clusterStateSettings.build(), + validator + ); + return task.execute(state); + } + + private void setupState() { + final ImmutableOpenMap.Builder stateCustomsBuilder = ImmutableOpenMap.builder(); + + currentStateMetaDataBuilder + .customs(customBuilder.build()) + .templates(tplBuilder.build()) + .indices(idxBuilder.build()); + + when(state.metaData()).thenReturn(currentStateMetaDataBuilder.build()); + + final ImmutableOpenMap.Builder> blockIdxBuilder = ImmutableOpenMap.builder(); + + when(currentStateBlocks.indices()).thenReturn(blockIdxBuilder.build()); + + when(state.blocks()).thenReturn(currentStateBlocks); + when(state.customs()).thenReturn(stateCustomsBuilder.build()); + when(state.routingTable()).thenReturn(routingTableBuilder.build()); + } + + private void setupRequest() { + when(request.settings()).thenReturn(reqSettings.build()); + when(request.index()).thenReturn("test"); + when(request.waitForActiveShards()).thenReturn(waitForActiveShardsNum); + when(request.blocks()).thenReturn(reqBlocks); + } + + private void setupClusterState() { + final DiscoveryNodes nodes = mock(DiscoveryNodes.class); + when(nodes.getSmallestNonClientNodeVersion()).thenReturn(Version.CURRENT); + + when(state.nodes()).thenReturn(nodes); + } + + @SuppressWarnings("unchecked") + private void setupIndicesService() throws Exception { + final RoutingFieldMapper routingMapper = mock(RoutingFieldMapper.class); + when(routingMapper.required()).thenReturn(false); + + when(docMapper.routingFieldMapper()).thenReturn(routingMapper); + when(docMapper.parentFieldMapper()).thenReturn(mock(ParentFieldMapper.class)); + + when(mapper.docMappers(anyBoolean())).thenReturn(Collections.singletonList(docMapper)); + + final Index index = new Index("target", "tgt1234"); + final Supplier supplier = mock(Supplier.class); + final IndexService service = mock(IndexService.class); + when(service.index()).thenReturn(index); + when(service.mapperService()).thenReturn(mapper); + when(service.getIndexSortSupplier()).thenReturn(supplier); + when(service.getIndexEventListener()).thenReturn(mock(IndexEventListener.class)); + + when(indicesService.createIndex(anyObject(), anyObject())).thenReturn(service); + } +} From 333f7ed1eda5c3be1843e27648e7c61641fdf6dd Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 08:48:02 +0300 Subject: [PATCH 3/7] reorder members of MetaDataCreateIndexService --- .../metadata/MetaDataCreateIndexService.java | 254 +++++++++--------- 1 file changed, 127 insertions(+), 127 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index ab679f312c59f..fd4e54f279885 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -233,115 +233,6 @@ private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, ); } - public void validateIndexSettings(String indexName, Settings settings) throws IndexCreationException { - List validationErrors = getIndexSettingsValidationErrors(settings); - if (validationErrors.isEmpty() == false) { - ValidationException validationException = new ValidationException(); - validationException.addValidationErrors(validationErrors); - throw new IndexCreationException(indexName, validationException); - } - } - - List getIndexSettingsValidationErrors(Settings settings) { - String customPath = IndexMetaData.INDEX_DATA_PATH_SETTING.get(settings); - List validationErrors = new ArrayList<>(); - if (Strings.isEmpty(customPath) == false && env.sharedDataFile() == null) { - validationErrors.add("path.shared_data must be set in order to use custom data paths"); - } else if (Strings.isEmpty(customPath) == false) { - Path resolvedPath = PathUtils.get(new Path[]{env.sharedDataFile()}, customPath); - if (resolvedPath == null) { - validationErrors.add("custom path [" + customPath + "] is not a sub-path of path.shared_data [" + env.sharedDataFile() + "]"); - } - } - return validationErrors; - } - - /** - * Validates the settings and mappings for shrinking an index. - * @return the list of nodes at least one instance of the source index shards are allocated - */ - static List validateShrinkIndex(ClusterState state, String sourceIndex, - Set targetIndexMappingsTypes, String targetIndexName, - Settings targetIndexSettings) { - if (state.metaData().hasIndex(targetIndexName)) { - throw new ResourceAlreadyExistsException(state.metaData().index(targetIndexName).getIndex()); - } - final IndexMetaData sourceMetaData = state.metaData().index(sourceIndex); - if (sourceMetaData == null) { - throw new IndexNotFoundException(sourceIndex); - } - // ensure index is read-only - if (state.blocks().indexBlocked(ClusterBlockLevel.WRITE, sourceIndex) == false) { - throw new IllegalStateException("index " + sourceIndex + " must be read-only to shrink index. use \"index.blocks.write=true\""); - } - - if (sourceMetaData.getNumberOfShards() == 1) { - throw new IllegalArgumentException("can't shrink an index with only one shard"); - } - - - if ((targetIndexMappingsTypes.size() > 1 || - (targetIndexMappingsTypes.isEmpty() || targetIndexMappingsTypes.contains(MapperService.DEFAULT_MAPPING)) == false)) { - throw new IllegalArgumentException("mappings are not allowed when shrinking indices" + - ", all mappings are copied from the source index"); - } - - if (IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings)) { - // this method applies all necessary checks ie. if the target shards are less than the source shards - // of if the source shards are divisible by the number of target shards - IndexMetaData.getRoutingFactor(sourceMetaData, IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings)); - } - - // now check that index is all on one node - final IndexRoutingTable table = state.routingTable().index(sourceIndex); - Map nodesToNumRouting = new HashMap<>(); - int numShards = sourceMetaData.getNumberOfShards(); - for (ShardRouting routing : table.shardsWithState(ShardRoutingState.STARTED)) { - nodesToNumRouting.computeIfAbsent(routing.currentNodeId(), (s) -> new AtomicInteger(0)).incrementAndGet(); - } - List nodesToAllocateOn = new ArrayList<>(); - for (Map.Entry entries : nodesToNumRouting.entrySet()) { - int numAllocations = entries.getValue().get(); - assert numAllocations <= numShards : "wait what? " + numAllocations + " is > than num shards " + numShards; - if (numAllocations == numShards) { - nodesToAllocateOn.add(entries.getKey()); - } - } - if (nodesToAllocateOn.isEmpty()) { - throw new IllegalStateException("index " + sourceIndex + - " must have all shards allocated on the same node to shrink index"); - } - return nodesToAllocateOn; - } - - static void prepareShrinkIndexSettings(ClusterState currentState, Set mappingKeys, Settings.Builder indexSettingsBuilder, Index shrinkFromIndex, String shrinkIntoName) { - final IndexMetaData sourceMetaData = currentState.metaData().index(shrinkFromIndex.getName()); - - final List nodesToAllocateOn = validateShrinkIndex(currentState, shrinkFromIndex.getName(), - mappingKeys, shrinkIntoName, indexSettingsBuilder.build()); - final Predicate sourceSettingsPredicate = (s) -> s.startsWith("index.similarity.") - || s.startsWith("index.analysis.") || s.startsWith("index.sort."); - indexSettingsBuilder - // we use "i.r.a.initial_recovery" rather than "i.r.a.require|include" since we want the replica to allocate right away - // once we are allocated. - .put(IndexMetaData.INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.getKey() + "_id", - Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray())) - // we only try once and then give up with a shrink index - .put("index.allocation.max_retries", 1) - // now copy all similarity / analysis / sort settings - this overrides all settings from the user unless they - // wanna add extra settings - .put(IndexMetaData.SETTING_VERSION_CREATED, sourceMetaData.getCreationVersion()) - .put(IndexMetaData.SETTING_VERSION_UPGRADED, sourceMetaData.getUpgradedVersion()) - .put(sourceMetaData.getSettings().filter(sourceSettingsPredicate)) - .put(IndexMetaData.SETTING_ROUTING_PARTITION_SIZE, sourceMetaData.getRoutingPartitionSize()) - .put(IndexMetaData.INDEX_SHRINK_SOURCE_NAME.getKey(), shrinkFromIndex.getName()) - .put(IndexMetaData.INDEX_SHRINK_SOURCE_UUID.getKey(), shrinkFromIndex.getUUID()); - } - - interface IndexValidator { - void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state); - } - static class IndexCreationTask extends AckedClusterStateUpdateTask { private final IndicesService indicesService; @@ -501,7 +392,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { if (shrinkFromIndex != null) { prepareShrinkIndexSettings( - currentState, mappings.keySet(), indexSettingsBuilder, shrinkFromIndex, request.index()); + currentState, mappings.keySet(), indexSettingsBuilder, shrinkFromIndex, request.index()); } final Settings actualIndexSettings = indexSettingsBuilder.build(); tmpImdBuilder.settings(actualIndexSettings); @@ -514,11 +405,11 @@ public ClusterState execute(ClusterState currentState) throws Exception { */ final IndexMetaData sourceMetaData = currentState.metaData().getIndexSafe(shrinkFromIndex); final long primaryTerm = - IntStream - .range(0, sourceMetaData.getNumberOfShards()) - .mapToLong(sourceMetaData::primaryTerm) - .max() - .getAsLong(); + IntStream + .range(0, sourceMetaData.getNumberOfShards()) + .mapToLong(sourceMetaData::primaryTerm) + .max() + .getAsLong(); for (int shardId = 0; shardId < tmpImdBuilder.numberOfShards(); shardId++) { tmpImdBuilder.primaryTerm(shardId, primaryTerm); } @@ -532,8 +423,8 @@ public ClusterState execute(ClusterState currentState) throws Exception { } if (waitForActiveShards.validate(tmpImd.getNumberOfReplicas()) == false) { throw new IllegalArgumentException("invalid wait_for_active_shards[" + request.waitForActiveShards() + - "]: cannot be greater than number of shard copies [" + - (tmpImd.getNumberOfReplicas() + 1) + "]"); + "]: cannot be greater than number of shard copies [" + + (tmpImd.getNumberOfReplicas() + 1) + "]"); } // create the index here (on the master) to validate it can be created, as well as adding the mapping final IndexService indexService = indicesService.createIndex(tmpImd, Collections.emptyList()); @@ -567,7 +458,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { for (AliasMetaData aliasMetaData : templatesAliases.values()) { if (aliasMetaData.filter() != null) { aliasValidator.validateAliasFilter(aliasMetaData.alias(), aliasMetaData.filter().uncompressed(), - queryShardContext, xContentRegistry); + queryShardContext, xContentRegistry); } } @@ -595,7 +486,7 @@ public ClusterState execute(ClusterState currentState) throws Exception { } for (Alias alias : request.aliases()) { AliasMetaData aliasMetaData = AliasMetaData.builder(alias.name()).filter(alias.filter()) - .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build(); + .indexRouting(alias.indexRouting()).searchRouting(alias.searchRouting()).build(); indexMetaDataBuilder.putAlias(aliasMetaData); } @@ -614,15 +505,15 @@ public ClusterState execute(ClusterState currentState) throws Exception { } indexService.getIndexEventListener().beforeIndexAddedToCluster(indexMetaData.getIndex(), - indexMetaData.getSettings()); + indexMetaData.getSettings()); MetaData newMetaData = MetaData.builder(currentState.metaData()) - .put(indexMetaData, false) - .build(); + .put(indexMetaData, false) + .build(); logger.info("[{}] creating index, cause [{}], templates {}, shards [{}]/[{}], mappings {}", - request.index(), request.cause(), templateNames, indexMetaData.getNumberOfShards(), - indexMetaData.getNumberOfReplicas(), mappings.keySet()); + request.index(), request.cause(), templateNames, indexMetaData.getNumberOfShards(), + indexMetaData.getNumberOfReplicas(), mappings.keySet()); ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks()); if (!request.blocks().isEmpty()) { @@ -636,10 +527,10 @@ public ClusterState execute(ClusterState currentState) throws Exception { if (request.state() == State.OPEN) { RoutingTable.Builder routingTableBuilder = RoutingTable.builder(updatedState.routingTable()) - .addAsNew(updatedState.metaData().index(request.index())); + .addAsNew(updatedState.metaData().index(request.index())); updatedState = allocationService.reroute( - ClusterState.builder(updatedState).routingTable(routingTableBuilder.build()).build(), - "index [" + request.index() + "] created"); + ClusterState.builder(updatedState).routingTable(routingTableBuilder.build()).build(), + "index [" + request.index() + "] created"); } removalExtraInfo = "cleaning up after validating index on master"; removalReason = IndexRemovalReason.NO_LONGER_ASSIGNED; @@ -678,4 +569,113 @@ private List findTemplates(CreateIndexClusterStateUpdateR return templateMetadata; } } + + public void validateIndexSettings(String indexName, Settings settings) throws IndexCreationException { + List validationErrors = getIndexSettingsValidationErrors(settings); + if (validationErrors.isEmpty() == false) { + ValidationException validationException = new ValidationException(); + validationException.addValidationErrors(validationErrors); + throw new IndexCreationException(indexName, validationException); + } + } + + List getIndexSettingsValidationErrors(Settings settings) { + String customPath = IndexMetaData.INDEX_DATA_PATH_SETTING.get(settings); + List validationErrors = new ArrayList<>(); + if (Strings.isEmpty(customPath) == false && env.sharedDataFile() == null) { + validationErrors.add("path.shared_data must be set in order to use custom data paths"); + } else if (Strings.isEmpty(customPath) == false) { + Path resolvedPath = PathUtils.get(new Path[]{env.sharedDataFile()}, customPath); + if (resolvedPath == null) { + validationErrors.add("custom path [" + customPath + "] is not a sub-path of path.shared_data [" + env.sharedDataFile() + "]"); + } + } + return validationErrors; + } + + /** + * Validates the settings and mappings for shrinking an index. + * @return the list of nodes at least one instance of the source index shards are allocated + */ + static List validateShrinkIndex(ClusterState state, String sourceIndex, + Set targetIndexMappingsTypes, String targetIndexName, + Settings targetIndexSettings) { + if (state.metaData().hasIndex(targetIndexName)) { + throw new ResourceAlreadyExistsException(state.metaData().index(targetIndexName).getIndex()); + } + final IndexMetaData sourceMetaData = state.metaData().index(sourceIndex); + if (sourceMetaData == null) { + throw new IndexNotFoundException(sourceIndex); + } + // ensure index is read-only + if (state.blocks().indexBlocked(ClusterBlockLevel.WRITE, sourceIndex) == false) { + throw new IllegalStateException("index " + sourceIndex + " must be read-only to shrink index. use \"index.blocks.write=true\""); + } + + if (sourceMetaData.getNumberOfShards() == 1) { + throw new IllegalArgumentException("can't shrink an index with only one shard"); + } + + + if ((targetIndexMappingsTypes.size() > 1 || + (targetIndexMappingsTypes.isEmpty() || targetIndexMappingsTypes.contains(MapperService.DEFAULT_MAPPING)) == false)) { + throw new IllegalArgumentException("mappings are not allowed when shrinking indices" + + ", all mappings are copied from the source index"); + } + + if (IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexSettings)) { + // this method applies all necessary checks ie. if the target shards are less than the source shards + // of if the source shards are divisible by the number of target shards + IndexMetaData.getRoutingFactor(sourceMetaData, IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.get(targetIndexSettings)); + } + + // now check that index is all on one node + final IndexRoutingTable table = state.routingTable().index(sourceIndex); + Map nodesToNumRouting = new HashMap<>(); + int numShards = sourceMetaData.getNumberOfShards(); + for (ShardRouting routing : table.shardsWithState(ShardRoutingState.STARTED)) { + nodesToNumRouting.computeIfAbsent(routing.currentNodeId(), (s) -> new AtomicInteger(0)).incrementAndGet(); + } + List nodesToAllocateOn = new ArrayList<>(); + for (Map.Entry entries : nodesToNumRouting.entrySet()) { + int numAllocations = entries.getValue().get(); + assert numAllocations <= numShards : "wait what? " + numAllocations + " is > than num shards " + numShards; + if (numAllocations == numShards) { + nodesToAllocateOn.add(entries.getKey()); + } + } + if (nodesToAllocateOn.isEmpty()) { + throw new IllegalStateException("index " + sourceIndex + + " must have all shards allocated on the same node to shrink index"); + } + return nodesToAllocateOn; + } + + static void prepareShrinkIndexSettings(ClusterState currentState, Set mappingKeys, Settings.Builder indexSettingsBuilder, Index shrinkFromIndex, String shrinkIntoName) { + final IndexMetaData sourceMetaData = currentState.metaData().index(shrinkFromIndex.getName()); + + final List nodesToAllocateOn = validateShrinkIndex(currentState, shrinkFromIndex.getName(), + mappingKeys, shrinkIntoName, indexSettingsBuilder.build()); + final Predicate sourceSettingsPredicate = (s) -> s.startsWith("index.similarity.") + || s.startsWith("index.analysis.") || s.startsWith("index.sort."); + indexSettingsBuilder + // we use "i.r.a.initial_recovery" rather than "i.r.a.require|include" since we want the replica to allocate right away + // once we are allocated. + .put(IndexMetaData.INDEX_ROUTING_INITIAL_RECOVERY_GROUP_SETTING.getKey() + "_id", + Strings.arrayToCommaDelimitedString(nodesToAllocateOn.toArray())) + // we only try once and then give up with a shrink index + .put("index.allocation.max_retries", 1) + // now copy all similarity / analysis / sort settings - this overrides all settings from the user unless they + // wanna add extra settings + .put(IndexMetaData.SETTING_VERSION_CREATED, sourceMetaData.getCreationVersion()) + .put(IndexMetaData.SETTING_VERSION_UPGRADED, sourceMetaData.getUpgradedVersion()) + .put(sourceMetaData.getSettings().filter(sourceSettingsPredicate)) + .put(IndexMetaData.SETTING_ROUTING_PARTITION_SIZE, sourceMetaData.getRoutingPartitionSize()) + .put(IndexMetaData.INDEX_SHRINK_SOURCE_NAME.getKey(), shrinkFromIndex.getName()) + .put(IndexMetaData.INDEX_SHRINK_SOURCE_UUID.getKey(), shrinkFromIndex.getUUID()); + } + + interface IndexValidator { + void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state); + } } From e3ee74db96e42ff22688eb145bce32ec825f044a Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 08:59:17 +0300 Subject: [PATCH 4/7] make diff on MetaDataCreateIndexService smaller --- .../metadata/MetaDataCreateIndexService.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index fd4e54f279885..b57e83e453903 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -223,14 +223,8 @@ private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, request.settings(updatedSettingsBuilder.build()); clusterService.submitStateUpdateTask("create-index [" + request.index() + "], cause [" + request.cause() + "]", - new IndexCreationTask(logger, allocationService, request, listener, - indicesService, aliasValidator, xContentRegistry, settings, - (req, state) -> { - validateIndexName(req.index(), state); - validateIndexSettings(req.index(), req.settings()); - } - ) - ); + new IndexCreationTask(logger, allocationService, request, listener, indicesService, aliasValidator, xContentRegistry, settings, + this::validate)); } static class IndexCreationTask extends AckedClusterStateUpdateTask { @@ -316,12 +310,12 @@ public ClusterState execute(ClusterState currentState) throws Exception { // handle custom for (ObjectObjectCursor cursor : template.customs()) { String type = cursor.key; - Custom custom = cursor.value; - Custom existing = customs.get(type); + IndexMetaData.Custom custom = cursor.value; + IndexMetaData.Custom existing = customs.get(type); if (existing == null) { customs.put(type, custom); } else { - Custom merged = existing.mergeWith(custom); + IndexMetaData.Custom merged = existing.mergeWith(custom); customs.put(type, merged); } } @@ -570,6 +564,11 @@ private List findTemplates(CreateIndexClusterStateUpdateR } } + private void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state) { + validateIndexName(request.index(), state); + validateIndexSettings(request.index(), request.settings()); + } + public void validateIndexSettings(String indexName, Settings settings) throws IndexCreationException { List validationErrors = getIndexSettingsValidationErrors(settings); if (validationErrors.isEmpty() == false) { From 2e615f01dcc0afdd6738f3eda251752c3656dec5 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sun, 30 Jul 2017 09:06:10 +0300 Subject: [PATCH 5/7] make diff on MetaDataCreateIndexService smaller --- .../cluster/metadata/MetaDataCreateIndexService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java index b57e83e453903..3e8ad550c0afd 100644 --- a/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java +++ b/core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java @@ -227,6 +227,10 @@ private void onlyCreateIndex(final CreateIndexClusterStateUpdateRequest request, this::validate)); } + interface IndexValidator { + void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state); + } + static class IndexCreationTask extends AckedClusterStateUpdateTask { private final IndicesService indicesService; @@ -673,8 +677,4 @@ static void prepareShrinkIndexSettings(ClusterState currentState, Set ma .put(IndexMetaData.INDEX_SHRINK_SOURCE_NAME.getKey(), shrinkFromIndex.getName()) .put(IndexMetaData.INDEX_SHRINK_SOURCE_UUID.getKey(), shrinkFromIndex.getUUID()); } - - interface IndexValidator { - void validate(CreateIndexClusterStateUpdateRequest request, ClusterState state); - } } From 3e0d7be0e36e84688fc4f8ed0c0ccfd5d2f99284 Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Sat, 26 Aug 2017 08:41:53 +0300 Subject: [PATCH 6/7] more descriptive matchers in index creation task test --- .../metadata/IndexCreationTaskTests.java | 130 ++++++++++++------ 1 file changed, 89 insertions(+), 41 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java index 8f2b1d6668c8f..093ce3c8ed5a4 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java @@ -50,9 +50,13 @@ import org.elasticsearch.index.shard.IndexEventListener; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.test.ESTestCase; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; import org.mockito.ArgumentCaptor; import java.io.IOException; +import java.util.List; import java.util.Map; import java.util.HashSet; import java.util.Set; @@ -61,6 +65,10 @@ import java.util.function.Supplier; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.not; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.mock; @@ -111,9 +119,8 @@ public void testMatchTemplates() throws Exception { final ClusterState result = executeTask(); - assertTrue(result.metaData().index("test").getAliases().containsKey("alias_from_template_1")); - assertTrue(result.metaData().index("test").getAliases().containsKey("alias_from_template_2")); - assertFalse(result.metaData().index("test").getAliases().containsKey("alias_from_template_3")); + assertThat(result.metaData().index("test").getAliases(), containsAllKeys("alias_from_template_1", "alias_from_template_2")); + assertThat(result.metaData().index("test").getAliases(), not(containsKey("alias_from_template_3"))); } public void testApplyDataFromTemplate() throws Exception { @@ -126,10 +133,10 @@ public void testApplyDataFromTemplate() throws Exception { final ClusterState result = executeTask(); - assertTrue(result.metaData().index("test").getAliases().containsKey("alias1")); - assertTrue(result.metaData().index("test").getCustoms().containsKey("custom1")); - assertEquals("value1", result.metaData().index("test").getSettings().get("key1")); - assertTrue(getMappingsFromResponse().containsKey("mapping1")); + assertThat(result.metaData().index("test").getAliases(), containsKey("alias1")); + assertThat(result.metaData().index("test").getCustoms(), containsKey("custom1")); + assertThat(result.metaData().index("test").getSettings().get("key1"), equalTo("value1")); + assertThat(getMappingsFromResponse(), hasKey("mapping1")); } public void testApplyDataFromRequest() throws Exception { @@ -140,16 +147,16 @@ public void testApplyDataFromRequest() throws Exception { final ClusterState result = executeTask(); - assertTrue(result.metaData().index("test").getAliases().containsKey("alias1")); - assertTrue(result.metaData().index("test").getCustoms().containsKey("custom1")); - assertEquals("value1", result.metaData().index("test").getSettings().get("key1")); - assertTrue(getMappingsFromResponse().containsKey("mapping1")); + assertThat(result.metaData().index("test").getAliases(), containsKey("alias1")); + assertThat(result.metaData().index("test").getCustoms(), containsKey("custom1")); + assertThat(result.metaData().index("test").getSettings().get("key1"), equalTo("value1")); + assertThat(getMappingsFromResponse(), hasKey("mapping1")); } public void testRequestDataHavePriorityOverTemplateData() throws Exception { - final IndexMetaData.Custom tplCustom = createCustom(); - final IndexMetaData.Custom reqCustom = createCustom(); - final IndexMetaData.Custom mergedCustom = createCustom(); + final IndexMetaData.Custom tplCustom = createCustom(); + final IndexMetaData.Custom reqCustom = createCustom(); + final IndexMetaData.Custom mergedCustom = createCustom(); when(reqCustom.mergeWith(tplCustom)).thenReturn(mergedCustom); final CompressedXContent tplMapping = createMapping("text"); @@ -169,16 +176,16 @@ public void testRequestDataHavePriorityOverTemplateData() throws Exception { final ClusterState result = executeTask(); - assertEquals(mergedCustom, result.metaData().index("test").getCustoms().get("custom1")); - assertEquals("fromReq", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); - assertEquals("reqValue", result.metaData().index("test").getSettings().get("key1")); - assertEquals("{type={properties={field={type=keyword}}}}", getMappingsFromResponse().get("mapping1").toString()); + assertThat(result.metaData().index("test").getCustoms().get("custom1"), equalTo(mergedCustom)); + assertThat(result.metaData().index("test").getAliases().get("alias1").getSearchRouting(), equalTo("fromReq")); + assertThat(result.metaData().index("test").getSettings().get("key1"), equalTo("reqValue")); + assertThat(getMappingsFromResponse().get("mapping1").toString(), equalTo("{type={properties={field={type=keyword}}}}")); } public void testDefaultSettings() throws Exception { final ClusterState result = executeTask(); - assertEquals("5", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + assertThat(result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS), equalTo("5")); } public void testSettingsFromClusterState() throws Exception { @@ -186,7 +193,7 @@ public void testSettingsFromClusterState() throws Exception { final ClusterState result = executeTask(); - assertEquals("15", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); + assertThat(result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS), equalTo("15")); } public void testTemplateOrder() throws Exception { @@ -207,8 +214,8 @@ public void testTemplateOrder() throws Exception { ); final ClusterState result = executeTask(); - assertEquals("12", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); - assertEquals("3", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); + assertThat(result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS), equalTo("12")); + assertThat(result.metaData().index("test").getAliases().get("alias1").getSearchRouting(), equalTo("3")); } public void testTemplateOrder2() throws Exception { @@ -229,12 +236,11 @@ public void testTemplateOrder2() throws Exception { ); final ClusterState result = executeTask(); - assertEquals("12", result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS)); - assertEquals("3", result.metaData().index("test").getAliases().get("alias1").getSearchRouting()); + assertThat(result.getMetaData().index("test").getSettings().get(SETTING_NUMBER_OF_SHARDS), equalTo("12")); + assertThat(result.metaData().index("test").getAliases().get("alias1").getSearchRouting(), equalTo("3")); } public void testRequestStateOpen() throws Exception { - when(request.state()).thenReturn(IndexMetaData.State.OPEN); executeTask(); @@ -246,12 +252,9 @@ public void testRequestStateOpen() throws Exception { public void testIndexRemovalOnFailure() throws Exception { doThrow(new RuntimeException("oops")).when(mapper).merge(anyMap(), anyObject(), anyBoolean()); - try { - executeTask(); - fail("exception not thrown"); - } catch (RuntimeException e) { - verify(indicesService, times(1)).removeIndex(anyObject(), anyObject(), anyObject()); - } + expectThrows(RuntimeException.class, this::executeTask); + + verify(indicesService, times(1)).removeIndex(anyObject(), anyObject(), anyObject()); } public void testShrinkIndexIgnoresTemplates() throws Exception { @@ -275,21 +278,18 @@ public void testShrinkIndexIgnoresTemplates() throws Exception { final ClusterState result = executeTask(); - assertFalse(result.metaData().index("test").getAliases().containsKey("alias1")); - assertFalse(result.metaData().index("test").getCustoms().containsKey("custom1")); - assertNull(result.metaData().index("test").getSettings().get("key1")); - assertFalse(getMappingsFromResponse().containsKey("mapping1")); + assertThat(result.metaData().index("test").getAliases(), not(containsKey("alias1"))); + assertThat(result.metaData().index("test").getCustoms(), not(containsKey("custom1"))); + assertThat(result.metaData().index("test").getSettings().getAsMap(), not(hasKey("key1"))); + assertThat(getMappingsFromResponse(), not(hasKey("mapping1"))); } public void testValidateWaitForActiveShardsFailure() throws Exception { waitForActiveShardsNum = ActiveShardCount.from(1000); - try { - executeTask(); - fail("validation exception expected"); - } catch (IllegalArgumentException e) { - assertTrue(e.getMessage().contains("invalid wait_for_active_shards")); - } + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, this::executeTask); + + assertThat(e.getMessage(), containsString("invalid wait_for_active_shards")); } private IndexRoutingTable createIndexRoutingTableWithStartedShards(Index index) { @@ -447,4 +447,52 @@ private void setupIndicesService() throws Exception { when(indicesService.createIndex(anyObject(), anyObject())).thenReturn(service); } + + private Matcher> containsKey(final K key) { + return new BaseMatcher>() { + + @Override + @SuppressWarnings("unchecked") + public boolean matches(final Object item) { + return ((ImmutableOpenMap)item).containsKey(key); + } + + @Override + public void describeTo(final Description description) { + description.appendText("ImmutableOpenMap should contain key [").appendValue(key).appendText("]"); + } + }; + } + + private Matcher> containsAllKeys(final K... keys) { + return new BaseMatcher>() { + + private K missingKey; + + @Override + @SuppressWarnings("unchecked") + public boolean matches(final Object item) { + final List expectedKeys = Arrays.asList(keys); + final ImmutableOpenMap map = ((ImmutableOpenMap)item); + + for (K key: expectedKeys) { + if (!map.containsKey(key)) { + missingKey = key; + return false; + } + } + + return true; + } + @Override + public void describeTo(final Description description) { + description + .appendText("ImmutableOpenMap should contain all keys ") + .appendValue(keys) + .appendText(", but key [") + .appendValue(missingKey) + .appendText("] is missing"); + } + }; + } } From f181dcb80280f78298899e686c9a0abb14cc837a Mon Sep 17 00:00:00 2001 From: Sergey Galkin Date: Mon, 28 Aug 2017 10:54:58 +0300 Subject: [PATCH 7/7] move custom matchers to test/framework/test/hamcrest/* --- .../metadata/IndexCreationTaskTests.java | 80 ++++--------------- .../test/hamcrest/CollectionAssertions.java | 4 + .../test/hamcrest/CollectionMatchers.java | 44 ++++++++++ 3 files changed, 63 insertions(+), 65 deletions(-) diff --git a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java index 093ce3c8ed5a4..9dc989961f32b 100644 --- a/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/metadata/IndexCreationTaskTests.java @@ -50,13 +50,10 @@ import org.elasticsearch.index.shard.IndexEventListener; import org.elasticsearch.indices.IndicesService; import org.elasticsearch.test.ESTestCase; -import org.hamcrest.BaseMatcher; -import org.hamcrest.Description; -import org.hamcrest.Matcher; +import org.hamcrest.Matchers; import org.mockito.ArgumentCaptor; import java.io.IOException; -import java.util.List; import java.util.Map; import java.util.HashSet; import java.util.Set; @@ -65,9 +62,10 @@ import java.util.function.Supplier; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; +import static org.elasticsearch.test.hamcrest.CollectionAssertions.hasAllKeys; +import static org.elasticsearch.test.hamcrest.CollectionAssertions.hasKey; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.not; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyObject; @@ -119,8 +117,8 @@ public void testMatchTemplates() throws Exception { final ClusterState result = executeTask(); - assertThat(result.metaData().index("test").getAliases(), containsAllKeys("alias_from_template_1", "alias_from_template_2")); - assertThat(result.metaData().index("test").getAliases(), not(containsKey("alias_from_template_3"))); + assertThat(result.metaData().index("test").getAliases(), hasAllKeys("alias_from_template_1", "alias_from_template_2")); + assertThat(result.metaData().index("test").getAliases(), not(hasKey("alias_from_template_3"))); } public void testApplyDataFromTemplate() throws Exception { @@ -133,10 +131,10 @@ public void testApplyDataFromTemplate() throws Exception { final ClusterState result = executeTask(); - assertThat(result.metaData().index("test").getAliases(), containsKey("alias1")); - assertThat(result.metaData().index("test").getCustoms(), containsKey("custom1")); + assertThat(result.metaData().index("test").getAliases(), hasKey("alias1")); + assertThat(result.metaData().index("test").getCustoms(), hasKey("custom1")); assertThat(result.metaData().index("test").getSettings().get("key1"), equalTo("value1")); - assertThat(getMappingsFromResponse(), hasKey("mapping1")); + assertThat(getMappingsFromResponse(), Matchers.hasKey("mapping1")); } public void testApplyDataFromRequest() throws Exception { @@ -147,10 +145,10 @@ public void testApplyDataFromRequest() throws Exception { final ClusterState result = executeTask(); - assertThat(result.metaData().index("test").getAliases(), containsKey("alias1")); - assertThat(result.metaData().index("test").getCustoms(), containsKey("custom1")); + assertThat(result.metaData().index("test").getAliases(), hasKey("alias1")); + assertThat(result.metaData().index("test").getCustoms(), hasKey("custom1")); assertThat(result.metaData().index("test").getSettings().get("key1"), equalTo("value1")); - assertThat(getMappingsFromResponse(), hasKey("mapping1")); + assertThat(getMappingsFromResponse(), Matchers.hasKey("mapping1")); } public void testRequestDataHavePriorityOverTemplateData() throws Exception { @@ -278,10 +276,10 @@ public void testShrinkIndexIgnoresTemplates() throws Exception { final ClusterState result = executeTask(); - assertThat(result.metaData().index("test").getAliases(), not(containsKey("alias1"))); - assertThat(result.metaData().index("test").getCustoms(), not(containsKey("custom1"))); - assertThat(result.metaData().index("test").getSettings().getAsMap(), not(hasKey("key1"))); - assertThat(getMappingsFromResponse(), not(hasKey("mapping1"))); + assertThat(result.metaData().index("test").getAliases(), not(hasKey("alias1"))); + assertThat(result.metaData().index("test").getCustoms(), not(hasKey("custom1"))); + assertThat(result.metaData().index("test").getSettings().getAsMap(), not(Matchers.hasKey("key1"))); + assertThat(getMappingsFromResponse(), not(Matchers.hasKey("mapping1"))); } public void testValidateWaitForActiveShardsFailure() throws Exception { @@ -447,52 +445,4 @@ private void setupIndicesService() throws Exception { when(indicesService.createIndex(anyObject(), anyObject())).thenReturn(service); } - - private Matcher> containsKey(final K key) { - return new BaseMatcher>() { - - @Override - @SuppressWarnings("unchecked") - public boolean matches(final Object item) { - return ((ImmutableOpenMap)item).containsKey(key); - } - - @Override - public void describeTo(final Description description) { - description.appendText("ImmutableOpenMap should contain key [").appendValue(key).appendText("]"); - } - }; - } - - private Matcher> containsAllKeys(final K... keys) { - return new BaseMatcher>() { - - private K missingKey; - - @Override - @SuppressWarnings("unchecked") - public boolean matches(final Object item) { - final List expectedKeys = Arrays.asList(keys); - final ImmutableOpenMap map = ((ImmutableOpenMap)item); - - for (K key: expectedKeys) { - if (!map.containsKey(key)) { - missingKey = key; - return false; - } - } - - return true; - } - @Override - public void describeTo(final Description description) { - description - .appendText("ImmutableOpenMap should contain all keys ") - .appendValue(keys) - .appendText(", but key [") - .appendValue(missingKey) - .appendText("] is missing"); - } - }; - } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionAssertions.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionAssertions.java index b21e94d30a720..2225a5d711e5c 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionAssertions.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionAssertions.java @@ -30,4 +30,8 @@ public class CollectionAssertions { public static Matcher hasKey(final String key) { return new CollectionMatchers.ImmutableOpenMapHasKeyMatcher(key); } + + public static Matcher hasAllKeys(final String... keys) { + return new CollectionMatchers.ImmutableOpenMapHasAllKeysMatcher(keys); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionMatchers.java b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionMatchers.java index 521ba58b0efc9..02a50d4164570 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionMatchers.java +++ b/test/framework/src/main/java/org/elasticsearch/test/hamcrest/CollectionMatchers.java @@ -22,6 +22,9 @@ import org.hamcrest.Description; import org.hamcrest.TypeSafeMatcher; +import java.util.Arrays; +import java.util.List; + /** * Matchers for easier handling of our custom collections, * for example ImmutableOpenMap @@ -56,4 +59,45 @@ public void describeTo(Description description) { } } + public static class ImmutableOpenMapHasAllKeysMatcher extends TypeSafeMatcher { + + private final List keys; + private String missingKey; + + public ImmutableOpenMapHasAllKeysMatcher(final String... keys) { + this.keys = Arrays.asList(keys); + } + + @Override + protected boolean matchesSafely(ImmutableOpenMap item) { + for (String key: keys) { + if (!item.containsKey(key)) { + missingKey = key; + return false; + } + } + + return true; + } + + @Override + public void describeMismatchSafely(final ImmutableOpenMap map, final Description mismatchDescription) { + if (map.size() == 0) { + mismatchDescription.appendText("was empty"); + } else { + mismatchDescription.appendText("was ").appendValue(map.keys()); + } + } + + @Override + public void describeTo(Description description) { + description + .appendText("ImmutableOpenMap should contain all keys ") + .appendValue(keys) + .appendText(", but key [") + .appendValue(missingKey) + .appendText("] is missing"); + } + } + }