Skip to content

Commit

Permalink
Merge remote-tracking branch 'elastic/8.15' into backport/8.15/pr-112582
Browse files Browse the repository at this point in the history
  • Loading branch information
jfreden committed Nov 22, 2024
2 parents 3b5ea5a + 9aaaaa5 commit 4db3098
Show file tree
Hide file tree
Showing 12 changed files with 141 additions and 69 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/116942.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 116942
summary: Fix handling of bulk requests with semantic text fields and delete ops
area: Relevance
type: bug
issues: []
6 changes: 6 additions & 0 deletions docs/changelog/117182.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 117182
summary: Change synthetic source logic for `constant_keyword`
area: Mapping
type: bug
issues:
- 117083
9 changes: 0 additions & 9 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ tests:
- class: org.elasticsearch.upgrades.FullClusterRestartIT
method: testSnapshotRestore {cluster=UPGRADED}
issue: https://github.com/elastic/elasticsearch/issues/111798
- class: org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidatorTests
method: testValidKebrerosTicket
issue: https://github.com/elastic/elasticsearch/issues/112632
- class: org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidatorTests
method: testKerbTicketGeneratedForDifferentServerFailsValidation
issue: https://github.com/elastic/elasticsearch/issues/112639
- class: org.elasticsearch.xpack.security.authc.kerberos.KerberosTicketValidatorTests
method: testWhenKeyTabWithInvalidContentFailsValidation
issue: https://github.com/elastic/elasticsearch/issues/112631
- class: org.elasticsearch.xpack.ml.integration.ClassificationIT
method: testDeleteExpiredData_RemovesUnusedState
issue: https://github.com/elastic/elasticsearch/issues/116234
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.action.bulk.BulkItemResponse;
import org.elasticsearch.action.bulk.BulkRequestBuilder;
import org.elasticsearch.action.bulk.BulkResponse;
import org.elasticsearch.action.delete.DeleteRequestBuilder;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
Expand All @@ -30,8 +31,10 @@
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

import static org.hamcrest.Matchers.equalTo;

Expand Down Expand Up @@ -86,30 +89,38 @@ public void testBulkOperations() throws Exception {

int totalBulkReqs = randomIntBetween(2, 100);
long totalDocs = 0;
Set<String> ids = new HashSet<>();
for (int bulkReqs = 0; bulkReqs < totalBulkReqs; bulkReqs++) {
BulkRequestBuilder bulkReqBuilder = client().prepareBulk();
int totalBulkSize = randomIntBetween(1, 100);
for (int bulkSize = 0; bulkSize < totalBulkSize; bulkSize++) {
String id = Long.toString(totalDocs);
if (ids.size() > 0 && rarely(random())) {
String id = randomFrom(ids);
ids.remove(id);
DeleteRequestBuilder request = new DeleteRequestBuilder(client(), INDEX_NAME).setId(id);
bulkReqBuilder.add(request);
continue;
}
String id = Long.toString(totalDocs++);
boolean isIndexRequest = randomBoolean();
Map<String, Object> source = new HashMap<>();
source.put("sparse_field", isIndexRequest && rarely() ? null : randomAlphaOfLengthBetween(0, 1000));
source.put("dense_field", isIndexRequest && rarely() ? null : randomAlphaOfLengthBetween(0, 1000));
if (isIndexRequest) {
bulkReqBuilder.add(new IndexRequestBuilder(client()).setIndex(INDEX_NAME).setId(id).setSource(source));
totalDocs++;
ids.add(id);
} else {
boolean isUpsert = randomBoolean();
UpdateRequestBuilder request = new UpdateRequestBuilder(client()).setIndex(INDEX_NAME).setDoc(source);
if (isUpsert || totalDocs == 0) {
if (isUpsert || ids.size() == 0) {
request.setDocAsUpsert(true);
totalDocs++;
} else {
// Update already existing document
id = Long.toString(randomLongBetween(0, totalDocs - 1));
id = randomFrom(ids);
}
request.setId(id);
bulkReqBuilder.add(request);
ids.add(id);
}
}
BulkResponse bulkResponse = bulkReqBuilder.get();
Expand All @@ -134,7 +145,7 @@ public void testBulkOperations() throws Exception {
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder().size(0).trackTotalHits(true);
SearchResponse searchResponse = client().search(new SearchRequest(INDEX_NAME).source(sourceBuilder)).get();
try {
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(totalDocs));
assertThat(searchResponse.getHits().getTotalHits().value, equalTo((long) ids.size()));
} finally {
searchResponse.decRef();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ public Set<NodeFeature> getFeatures() {

@Override
public Set<NodeFeature> getTestFeatures() {
return Set.of(SemanticTextFieldMapper.SEMANTIC_TEXT_IN_OBJECT_FIELD_FIX, SemanticTextFieldMapper.SEMANTIC_TEXT_ZERO_SIZE_FIX);
return Set.of(
SemanticTextFieldMapper.SEMANTIC_TEXT_IN_OBJECT_FIELD_FIX,
SemanticTextFieldMapper.SEMANTIC_TEXT_ZERO_SIZE_FIX,
SemanticTextFieldMapper.SEMANTIC_TEXT_DELETE_FIX
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -412,8 +412,8 @@ private void applyInferenceResponses(BulkItemRequest item, FieldInferenceRespons
*/
private Map<String, List<FieldInferenceRequest>> createFieldInferenceRequests(BulkShardRequest bulkShardRequest) {
Map<String, List<FieldInferenceRequest>> fieldRequestsMap = new LinkedHashMap<>();
int itemIndex = 0;
for (var item : bulkShardRequest.items()) {
for (int itemIndex = 0; itemIndex < bulkShardRequest.items().length; itemIndex++) {
var item = bulkShardRequest.items()[itemIndex];
if (item.getPrimaryResponse() != null) {
// item was already aborted/processed by a filter in the chain upstream (e.g. security)
continue;
Expand All @@ -440,6 +440,7 @@ private Map<String, List<FieldInferenceRequest>> createFieldInferenceRequests(Bu
// ignore delete request
continue;
}

final Map<String, Object> docMap = indexRequest.sourceAsMap();
for (var entry : fieldInferenceMap.values()) {
String field = entry.getName();
Expand Down Expand Up @@ -481,7 +482,6 @@ private Map<String, List<FieldInferenceRequest>> createFieldInferenceRequests(Bu
}
}
}
itemIndex++;
}
return fieldRequestsMap;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
public class SemanticTextFieldMapper extends FieldMapper implements InferenceFieldMapper {
public static final NodeFeature SEMANTIC_TEXT_IN_OBJECT_FIELD_FIX = new NodeFeature("semantic_text.in_object_field_fix");
public static final NodeFeature SEMANTIC_TEXT_ZERO_SIZE_FIX = new NodeFeature("semantic_text.zero_size_fix");
public static final NodeFeature SEMANTIC_TEXT_DELETE_FIX = new NodeFeature("semantic_text.delete_fix");

public static final String CONTENT_TYPE = "semantic_text";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,3 +502,59 @@ setup:
- match: { _source.level_1.dense_field.text: "another inference test" }
- exists: _source.level_1.dense_field.inference.chunks.0.embeddings
- match: { _source.level_1.dense_field.inference.chunks.0.text: "another inference test" }

---
"Deletes on bulk operation":
- requires:
cluster_features: semantic_text.delete_fix
reason: Delete operations are properly applied when subsequent operations include a semantic text field.

- do:
bulk:
index: test-index
refresh: true
body: |
{"index":{"_id": "1"}}
{"dense_field": ["you know, for testing", "now with chunks"]}
{"index":{"_id": "2"}}
{"dense_field": ["some more tests", "that include chunks"]}
- do:
search:
index: test-index
body:
query:
semantic:
field: dense_field
query: "you know, for testing"

- match: { hits.total.value: 2 }
- match: { hits.total.relation: eq }
- match: { hits.hits.0._source.dense_field.text: ["you know, for testing", "now with chunks"] }
- match: { hits.hits.1._source.dense_field.text: ["some more tests", "that include chunks"] }

- do:
bulk:
index: test-index
refresh: true
body: |
{"delete":{ "_id": "2"}}
{"update":{"_id": "1"}}
{"doc":{"dense_field": "updated text", "sparse_field": "another text"}}
- match: { errors: false }


- do:
search:
index: test-index
body:
query:
semantic:
field: dense_field
query: "you know, for testing"

- match: { hits.total.value: 1 }
- match: { hits.total.relation: eq }
- match: { hits.hits.0._source.dense_field.text: "updated text" }
- match: { hits.hits.0._source.sparse_field.text: "another text" }
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
package org.elasticsearch.xpack.constantkeyword.mapper;

import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.LeafReader;
import org.apache.lucene.index.TermsEnum;
import org.apache.lucene.search.MatchAllDocsQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
Expand Down Expand Up @@ -41,7 +40,6 @@
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperBuilderContext;
import org.elasticsearch.index.mapper.MapperParsingException;
import org.elasticsearch.index.mapper.SourceLoader;
import org.elasticsearch.index.mapper.ValueFetcher;
import org.elasticsearch.index.query.QueryRewriteContext;
import org.elasticsearch.index.query.SearchExecutionContext;
Expand All @@ -57,7 +55,6 @@
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;

/**
* A {@link FieldMapper} that assigns every document the same value.
Expand Down Expand Up @@ -345,48 +342,14 @@ protected String contentType() {

@Override
protected SyntheticSourceMode syntheticSourceMode() {
return SyntheticSourceMode.NATIVE;
}

@Override
public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
String value = fieldType().value();
;
if (value == null) {
return SourceLoader.SyntheticFieldLoader.NOTHING;
}
return new SourceLoader.SyntheticFieldLoader() {
@Override
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
return Stream.of();
}

@Override
public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) {
return docId -> true;
}

@Override
public boolean hasValue() {
return true;
}

@Override
public void write(XContentBuilder b) throws IOException {
if (fieldType().value != null) {
b.field(leafName(), fieldType().value);
}
}

@Override
public void reset() {
// NOOP
}

@Override
public String fieldName() {
return fullPath();
}
};
/*
If there was no value in the document, synthetic source should not have the value too.
This is consistent with stored source behavior and is important for scenarios
like reindexing into an index that has a different value of this value in the mapping.
In order to do that we use fallback logic which implements exactly such logic (_source only contains value
if it was in the original document).
*/
return SyntheticSourceMode.FALLBACK;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,17 @@ public void testNullValueSyntheticSource() throws IOException {
assertThat(syntheticSource(mapper, b -> {}), equalTo("{}"));
}

public void testNoValueInDocumentSyntheticSource() throws IOException {
DocumentMapper mapper = createDocumentMapper(syntheticSourceMapping(b -> {
b.startObject("field");
b.field("type", "constant_keyword");
b.field("value", randomAlphaOfLength(5));
b.endObject();
}));

assertThat(syntheticSource(mapper, b -> {}), equalTo("{}"));
}

@Override
protected boolean supportsEmptyInputArray() {
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
constant_keyword:
- requires:
cluster_features: ["gte_v8.4.0"]
reason: introduced in 8.4.0
cluster_features: "gte_v8.15.0"
reason: behavior change in 8.15

- do:
indices.create:
Expand All @@ -25,13 +25,35 @@ constant_keyword:
body:
kwd: foo

- do:
index:
index: test
id: 2
refresh: true
body:
kwd: foo
const_kwd: bar

- do:
search:
index: test
body:
query:
ids:
values: [1]

- match:
hits.hits.0._source:
kwd: foo

- do:
search:
index: test
body:
query:
ids:
values: [2]

- match:
hits.hits.0._source:
kwd: foo
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public abstract class KerberosTestCase extends ESTestCase {
*
* Note: several unsupported locales were added in CLDR. #109670 included these below.
*/
private static Set<String> UNSUPPORTED_LOCALE_LANGUAGES = Set.of(
private static final Set<String> UNSUPPORTED_LOCALE_LANGUAGES = Set.of(
"ar",
"ja",
"th",
Expand All @@ -88,7 +88,9 @@ public abstract class KerberosTestCase extends ESTestCase {
"sat",
"sa",
"bgc",
"raj"
"raj",
"nqo",
"bho"
);

@BeforeClass
Expand Down

0 comments on commit 4db3098

Please sign in to comment.