Skip to content

Commit

Permalink
fix(elasticsearch): Upgrading to ElasticSearch 6 for entity tags (#4386)
Browse files Browse the repository at this point in the history 
This change follows up on the change #2913.
* Remove ES2.x support - now all docs using mapping type `_doc` recommended by ES6 and only one is allowed.
* Change the queries to use `entityRef.entityType` instead of the document type.
* Fixup ES template to not use `not_analyzed` but instead use `keyword` (with exception of `entityRef.entityType` which we wildcard query)
* Adding an `id` field to the document because the built-in `_id` field is no longer wildcard queryable
* Changing to use a container for the unittests
  • Loading branch information
@marchello2000
marchello2000 committed Mar 10, 2020
1 parent dcf1828 commit fe1ab9b
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 118 deletions.
5 changes: 3 additions & 2 deletions clouddriver-elasticsearch/clouddriver-elasticsearch.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ dependencies {
implementation "com.netflix.spinnaker.kork:kork-security"
implementation "com.squareup.retrofit:retrofit"
implementation "org.codehaus.groovy:groovy-all"
implementation("org.elasticsearch:elasticsearch:2.4.1") {
implementation("org.elasticsearch:elasticsearch:6.8.6") {
force = true
}

implementation("io.searchbox:jest:2.0.3") {
implementation("io.searchbox:jest:6.3.1") {
force = true
}
implementation "org.springframework.boot:spring-boot-starter-web"

testCompile "org.testcontainers:elasticsearch:1.12.5"
testImplementation "cglib:cglib-nodep"
testImplementation "org.objenesis:objenesis"
testImplementation "org.spockframework:spock-core"
Expand Down
44 changes: 19 additions & 25 deletions clouddriver-elasticsearch/elasticsearch_index_template.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"order": 0,
"template": "tags_v*",
"index_patterns": [
"tags_v*"
],
"settings": {
"index": {
"number_of_shards": "6",
Expand All @@ -9,7 +11,7 @@
}
},
"mappings": {
"_default_": {
"_doc": {
"dynamic": "false",
"dynamic_templates": [
{
Expand All @@ -24,62 +26,54 @@
"entityRef_template": {
"path_match": "entityRef.*",
"mapping": {
"index": "not_analyzed"
"index": "keyword"
}
}
}
],
"properties": {
"id": {
"type": "text"
},
"entityRef": {
"properties": {
"accountId": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"application": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"entityType": {
"index": "not_analyzed",
"type": "string"
"type": "text"
},
"cloudProvider": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"entityId": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"region": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"account": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
}
}
},
"tags": {
"type": "nested",
"properties": {
"valueType": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"name": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"namespace": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
},
"value": {
"index": "not_analyzed",
"type": "string"
"type": "keyword"
}
}
}
Expand Down
74 changes: 29 additions & 45 deletions ...om/netflix/spinnaker/clouddriver/elasticsearch/model/ElasticSearchEntityTagsProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import java.io.IOException;
import java.util.*;
import java.util.stream.Collectors;
import org.apache.lucene.search.join.ScoreMode;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
Expand All @@ -64,7 +65,6 @@ public class ElasticSearchEntityTagsProvider implements EntityTagsProvider {

private final String activeElasticSearchIndex;

private final boolean singleMappingType;
private final String mappingTypeName;

@Autowired
Expand All @@ -81,7 +81,6 @@ public ElasticSearchEntityTagsProvider(
this.front50Service = front50Service;
this.jestClient = jestClient;
this.activeElasticSearchIndex = elasticSearchConfigProperties.getActiveIndex();
this.singleMappingType = elasticSearchConfigProperties.isSingleMappingType();
this.mappingTypeName = elasticSearchConfigProperties.getMappingTypeName();
}

Expand Down Expand Up @@ -131,6 +130,11 @@ public Collection<EntityTags> getAll(
queryBuilder = queryBuilder.must(QueryBuilders.wildcardQuery("id", idPrefix));
}

if (entityType != null) {
queryBuilder =
queryBuilder.must(QueryBuilders.wildcardQuery("entityRef.entityType", entityType));
}

if (tags != null) {
for (Map.Entry<String, Object> entry : tags.entrySet()) {
// each key/value pair maps to a distinct nested `tags` object and must be a unique query
Expand All @@ -150,7 +154,7 @@ public Collection<EntityTags> getAll(
queryBuilder = queryBuilder.must(applyTagsToBuilder(namespace, Collections.emptyMap()));
}

return search(entityType, queryBuilder, maxResults);
return search(queryBuilder, maxResults);
}

@Override
Expand All @@ -173,7 +177,7 @@ public Optional<EntityTags> get(String id, Map<String, Object> tags) {
}
}

List<EntityTags> entityTags = search(null, queryBuilder, 1);
List<EntityTags> entityTags = search(queryBuilder, 1);
return entityTags.isEmpty() ? Optional.empty() : Optional.of(entityTags.get(0));
}

Expand All @@ -184,7 +188,7 @@ public void index(EntityTags entityTags) {
new Index.Builder(
objectMapper.convertValue(prepareForWrite(objectMapper, entityTags), Map.class))
.index(activeElasticSearchIndex)
.type(getDocumentType(entityTags))
.type(mappingTypeName)
.id(entityTags.getId())
.build();

Expand All @@ -209,13 +213,13 @@ public void bulkIndex(Collection<EntityTags> multipleEntityTags) {
Bulk.Builder builder = new Bulk.Builder().defaultIndex(activeElasticSearchIndex);

for (EntityTags entityTags : tags) {
Map tag =
objectMapper.convertValue(prepareForWrite(objectMapper, entityTags), Map.class);
builder =
builder.addAction(
new Index.Builder(
objectMapper.convertValue(
prepareForWrite(objectMapper, entityTags), Map.class))
new Index.Builder(tag)
.index(activeElasticSearchIndex)
.type(getDocumentType(entityTags))
.type(mappingTypeName)
.id(entityTags.getId())
.build());
}
Expand Down Expand Up @@ -255,10 +259,7 @@ public void delete(String id) {
}

Delete action =
new Delete.Builder(id)
.index(activeElasticSearchIndex)
.type(getDocumentType(entityTags))
.build();
new Delete.Builder(id).index(activeElasticSearchIndex).type(mappingTypeName).build();

JestResult jestResult = jestClient.execute(action);
if (!jestResult.isSucceeded()) {
Expand All @@ -281,9 +282,7 @@ public void bulkDelete(Collection<EntityTags> multipleEntityTags) {
for (EntityTags entityTags : tags) {
builder =
builder.addAction(
new Delete.Builder(entityTags.getId())
.type(getDocumentType(entityTags))
.build());
new Delete.Builder(entityTags.getId()).type(mappingTypeName).build());
}

Bulk bulk = builder.build();
Expand Down Expand Up @@ -352,9 +351,14 @@ public Map delta() {
entityTagsByEntityTypeFront50
.keySet()
.forEach(
entityType ->
entityTagsByEntityTypeElasticsearch.put(
entityType, fetchAll(QueryBuilders.matchAllQuery(), entityType, 5000, "2m")));
entityType -> {
BoolQueryBuilder queryBuilder = QueryBuilders.boolQuery();
queryBuilder =
queryBuilder.must(QueryBuilders.termQuery("entityRef.entityType", entityType));

entityTagsByEntityTypeElasticsearch.put(
entityType, fetchAll(queryBuilder, 5000, "2m"));
});

Map<String, Map> metadata = new HashMap<>();

Expand Down Expand Up @@ -566,7 +570,7 @@ private List<EntityTags> getAllMatchingEntityTags(String namespace, String tag)
QueryBuilders.boolQuery()
.must(applyTagsToBuilder(null, Collections.singletonMap(tag, "*")));

fetchAll(queryBuilder, null, 5000, "2m")
fetchAll(queryBuilder, 5000, "2m")
.forEach(
entityTags -> {
if (!entityTagsIdentifiers.contains(entityTags.getId())) {
Expand All @@ -580,7 +584,7 @@ private List<EntityTags> getAllMatchingEntityTags(String namespace, String tag)
BoolQueryBuilder queryBuilder =
QueryBuilders.boolQuery().must(applyTagsToBuilder(namespace, Collections.emptyMap()));

fetchAll(queryBuilder, null, 5000, "2m")
fetchAll(queryBuilder, 5000, "2m")
.forEach(
entityTags -> {
if (!entityTagsIdentifiers.contains(entityTags.getId())) {
Expand Down Expand Up @@ -610,7 +614,7 @@ private QueryBuilder applyTagsToBuilder(String namespace, Map<String, Object> ta
boolQueryBuilder.must(QueryBuilders.termQuery("tags.namespace", namespace));
}

return QueryBuilders.nestedQuery("tags", boolQueryBuilder);
return QueryBuilders.nestedQuery("tags", boolQueryBuilder, ScoreMode.Avg);
}

/** Elasticsearch requires that all search criteria be flattened (vs. nested) */
Expand All @@ -628,17 +632,13 @@ private Map<String, Object> flatten(
return accumulator;
}

private List<EntityTags> search(String type, QueryBuilder queryBuilder, int maxResults) {
private List<EntityTags> search(QueryBuilder queryBuilder, int maxResults) {
SearchSourceBuilder searchSourceBuilder =
new SearchSourceBuilder().query(queryBuilder).size(maxResults);
String searchQuery = searchSourceBuilder.toString();

Search.Builder searchBuilder =
new Search.Builder(searchQuery).addIndex(activeElasticSearchIndex);
if (type != null) {
// restrict to a specific index type (optional)
searchBuilder.addType(type.toLowerCase());
}

try {
SearchResult searchResult = jestClient.execute(searchBuilder.build());
Expand All @@ -651,18 +651,13 @@ private List<EntityTags> search(String type, QueryBuilder queryBuilder, int maxR
}
}

private List<EntityTags> fetchAll(
QueryBuilder queryBuilder, String type, int scrollSize, String scrollTime) {
private List<EntityTags> fetchAll(QueryBuilder queryBuilder, int scrollSize, String scrollTime) {
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder();
searchSourceBuilder.query(queryBuilder);

Search.Builder builder =
new Search.Builder(searchSourceBuilder.toString()).addIndex(activeElasticSearchIndex);

if (type != null) {
builder.addType(type);
}

Search search =
builder
.setParameter(Parameters.SIZE, scrollSize)
Expand All @@ -684,10 +679,7 @@ private List<EntityTags> fetchAll(

try {
while (entityTags.size() > 0) {
SearchScroll scroll =
new SearchScroll.Builder(scrollId, scrollTime)
.setParameter(Parameters.SIZE, scrollSize)
.build();
SearchScroll scroll = new SearchScroll.Builder(scrollId, scrollTime).build();

try {
result = jestClient.execute(scroll);
Expand Down Expand Up @@ -748,12 +740,4 @@ private static EntityTags prepareForRead(ObjectMapper objectMapper, Map indexedE

return entityTags;
}

private String getDocumentType(EntityTags entityTags) {
if (singleMappingType) {
return mappingTypeName;
} else {
return entityTags.getEntityRef().getEntityType();
}
}
}
9 changes: 0 additions & 9 deletions ...asticsearch/src/main/java/com/netflix/spinnaker/config/ElasticSearchConfigProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public class ElasticSearchConfigProperties {
// The name of the unique mapping type is configurable as mappingTypeName, but is defaulted to
// "_doc", which is
// recommended for forward compatibility with Elasticsearch 7.0.
private boolean singleMappingType = false;
private String mappingTypeName = "_doc";

public String getActiveIndex() {
Expand Down Expand Up @@ -69,14 +68,6 @@ public void setConnectionTimeout(int connectionTimeout) {
this.connectionTimeout = connectionTimeout;
}

public void setSingleMappingType(boolean singleMappingType) {
this.singleMappingType = singleMappingType;
}

public boolean isSingleMappingType() {
return singleMappingType;
}

public void setMappingTypeName(String mappingTypeName) {
this.mappingTypeName = mappingTypeName;
}
Expand Down
Loading

0 comments on commit fe1ab9b

Please sign in to comment.