Skip to content

Commit

Permalink
Merge branch 'main' into jmh_upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
elasticmachine authored Jul 8, 2024
2 parents b2cd279 + b01949c commit 257028f
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 56 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/110554.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 110554
summary: Fix `MapperBuilderContext#isDataStream` when used in dynamic mappers
area: "Mapping"
type: bug
issues: []
14 changes: 10 additions & 4 deletions muted-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ tests:
method: "testGuessIsDayFirstFromLocale"
- class: "org.elasticsearch.test.rest.ClientYamlTestSuiteIT"
issue: "https://github.com/elastic/elasticsearch/issues/108857"
method: "test {yaml=search/180_locale_dependent_mapping/Test Index and Search locale dependent mappings / dates}"
method: "test {yaml=search/180_locale_dependent_mapping/Test Index and Search locale\
\ dependent mappings / dates}"
- class: "org.elasticsearch.upgrades.SearchStatesIT"
issue: "https://github.com/elastic/elasticsearch/issues/108991"
method: "testCanMatch"
Expand All @@ -13,7 +14,8 @@ tests:
method: "testTrainedModelInference"
- class: "org.elasticsearch.xpack.security.CoreWithSecurityClientYamlTestSuiteIT"
issue: "https://github.com/elastic/elasticsearch/issues/109188"
method: "test {yaml=search/180_locale_dependent_mapping/Test Index and Search locale dependent mappings / dates}"
method: "test {yaml=search/180_locale_dependent_mapping/Test Index and Search locale\
\ dependent mappings / dates}"
- class: "org.elasticsearch.xpack.esql.qa.mixed.EsqlClientYamlIT"
issue: "https://github.com/elastic/elasticsearch/issues/109189"
method: "test {p0=esql/70_locale/Date format with Italian locale}"
Expand All @@ -28,7 +30,8 @@ tests:
method: "testTimestampFieldTypeExposedByAllIndicesServices"
- class: "org.elasticsearch.analysis.common.CommonAnalysisClientYamlTestSuiteIT"
issue: "https://github.com/elastic/elasticsearch/issues/109318"
method: "test {yaml=analysis-common/50_char_filters/pattern_replace error handling (too complex pattern)}"
method: "test {yaml=analysis-common/50_char_filters/pattern_replace error handling\
\ (too complex pattern)}"
- class: "org.elasticsearch.xpack.ml.integration.ClassificationHousePricingIT"
issue: "https://github.com/elastic/elasticsearch/issues/101598"
method: "testFeatureImportanceValues"
Expand Down Expand Up @@ -95,8 +98,11 @@ tests:
issue: "https://github.com/elastic/elasticsearch/issues/110408"
method: "testCreateAndRestorePartialSearchableSnapshot"
- class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT
method: test {p0=search.vectors/41_knn_search_half_byte_quantized/Test create, merge, and search cosine}
method: test {p0=search.vectors/41_knn_search_half_byte_quantized/Test create, merge,
and search cosine}
issue: https://github.com/elastic/elasticsearch/issues/109978
- class: "org.elasticsearch.xpack.esql.qa.mixed.MixedClusterEsqlSpecIT"
issue: "https://github.com/elastic/elasticsearch/issues/110591"

# Examples:
#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ public final MapperBuilderContext createDynamicMapperBuilderContext() {
return new MapperBuilderContext(
p,
mappingLookup.isSourceSynthetic(),
false,
mappingLookup.isDataStreamTimestampFieldEnabled(),
containsDimensions,
dynamic,
MergeReason.MAPPING_UPDATE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.IndexVersion;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xcontent.XContentBuilder;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentType;
import org.elasticsearch.xcontent.json.JsonXContent;

import java.io.IOException;
Expand Down Expand Up @@ -81,4 +83,54 @@ public void testSwitchParser() throws IOException {
assertEquals(parser, newContext.parser());
assertEquals("1", newContext.indexSettings().getSettings().get("index.mapping.total_fields.limit"));
}

public void testCreateDynamicMapperBuilderContextFromEmptyContext() throws IOException {
var resultFromEmptyParserContext = context.createDynamicMapperBuilderContext();

assertEquals("hey", resultFromEmptyParserContext.buildFullName("hey"));
assertFalse(resultFromEmptyParserContext.isSourceSynthetic());
assertFalse(resultFromEmptyParserContext.isDataStream());
assertFalse(resultFromEmptyParserContext.parentObjectContainsDimensions());
assertEquals(ObjectMapper.Defaults.DYNAMIC, resultFromEmptyParserContext.getDynamic());
assertEquals(MapperService.MergeReason.MAPPING_UPDATE, resultFromEmptyParserContext.getMergeReason());
assertFalse(resultFromEmptyParserContext.isInNestedContext());
}

public void testCreateDynamicMapperBuilderContext() throws IOException {
var mapping = XContentBuilder.builder(XContentType.JSON.xContent())
.startObject()
.startObject("_doc")
.startObject("_source")
.field("mode", "synthetic")
.endObject()
.startObject(DataStreamTimestampFieldMapper.NAME)
.field("enabled", "true")
.endObject()
.startObject("properties")
.startObject(DataStreamTimestampFieldMapper.DEFAULT_PATH)
.field("type", "date")
.endObject()
.startObject("foo")
.field("type", "passthrough")
.field("time_series_dimension", "true")
.field("priority", "100")
.endObject()
.endObject()
.endObject()
.endObject();
var documentMapper = new MapperServiceTestCase() {
}.createDocumentMapper(mapping);
var parserContext = new TestDocumentParserContext(documentMapper.mappers(), null);
parserContext.path().add("foo");

var resultFromParserContext = parserContext.createDynamicMapperBuilderContext();

assertEquals("foo.hey", resultFromParserContext.buildFullName("hey"));
assertTrue(resultFromParserContext.isSourceSynthetic());
assertTrue(resultFromParserContext.isDataStream());
assertTrue(resultFromParserContext.parentObjectContainsDimensions());
assertEquals(ObjectMapper.Defaults.DYNAMIC, resultFromParserContext.getDynamic());
assertEquals(MapperService.MergeReason.MAPPING_UPDATE, resultFromParserContext.getMergeReason());
assertFalse(resultFromParserContext.isInNestedContext());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@
import org.elasticsearch.inference.ModelSecrets;
import org.elasticsearch.inference.TaskType;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.core.inference.results.ErrorChunkedInferenceResults;
import org.elasticsearch.xpack.core.inference.results.InferenceChunkedTextEmbeddingFloatResults;
import org.elasticsearch.xpack.core.inference.results.InferenceTextEmbeddingFloatResults;
import org.elasticsearch.xpack.core.ml.inference.results.ErrorInferenceResults;
import org.elasticsearch.xpack.inference.chunking.EmbeddingRequestChunker;
import org.elasticsearch.xpack.inference.external.action.amazonbedrock.AmazonBedrockActionCreator;
import org.elasticsearch.xpack.inference.external.amazonbedrock.AmazonBedrockRequestSender;
Expand All @@ -47,7 +43,6 @@
import java.util.Set;

import static org.elasticsearch.TransportVersions.ML_INFERENCE_AMAZON_BEDROCK_ADDED;
import static org.elasticsearch.xpack.core.inference.results.ResultUtils.createInvalidChunkedResultException;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.createInvalidModelException;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.parsePersistedConfigErrorMsg;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.removeFromMapOrDefaultEmpty;
Expand Down Expand Up @@ -115,37 +110,20 @@ protected void doChunkedInfer(
TimeValue timeout,
ActionListener<List<ChunkedInferenceServiceResults>> listener
) {
ActionListener<InferenceServiceResults> inferListener = listener.delegateFailureAndWrap(
(delegate, response) -> delegate.onResponse(translateToChunkedResults(input, response))
);

var actionCreator = new AmazonBedrockActionCreator(amazonBedrockSender, this.getServiceComponents(), timeout);
if (model instanceof AmazonBedrockModel baseAmazonBedrockModel) {
var maxBatchSize = getEmbeddingsMaxBatchSize(baseAmazonBedrockModel.provider());
var batchedRequests = new EmbeddingRequestChunker(input, maxBatchSize, EmbeddingRequestChunker.EmbeddingType.FLOAT)
.batchRequestsWithListeners(listener);
for (var request : batchedRequests) {
var action = baseAmazonBedrockModel.accept(actionCreator, taskSettings);
action.execute(new DocumentsOnlyInput(request.batch().inputs()), timeout, inferListener);
action.execute(new DocumentsOnlyInput(request.batch().inputs()), timeout, request.listener());
}
} else {
listener.onFailure(createInvalidModelException(model));
}
}

private static List<ChunkedInferenceServiceResults> translateToChunkedResults(
List<String> inputs,
InferenceServiceResults inferenceResults
) {
if (inferenceResults instanceof InferenceTextEmbeddingFloatResults textEmbeddingResults) {
return InferenceChunkedTextEmbeddingFloatResults.listOf(inputs, textEmbeddingResults);
} else if (inferenceResults instanceof ErrorInferenceResults error) {
return List.of(new ErrorChunkedInferenceResults(error.getException()));
} else {
throw createInvalidChunkedResultException(InferenceTextEmbeddingFloatResults.NAME, inferenceResults.getWriteableName());
}
}

@Override
public String name() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@
import org.elasticsearch.inference.SimilarityMeasure;
import org.elasticsearch.inference.TaskType;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.xpack.core.inference.results.ErrorChunkedInferenceResults;
import org.elasticsearch.xpack.core.inference.results.InferenceChunkedTextEmbeddingFloatResults;
import org.elasticsearch.xpack.core.inference.results.InferenceTextEmbeddingFloatResults;
import org.elasticsearch.xpack.core.ml.inference.results.ErrorInferenceResults;
import org.elasticsearch.xpack.inference.chunking.EmbeddingRequestChunker;
import org.elasticsearch.xpack.inference.external.action.azureopenai.AzureOpenAiActionCreator;
import org.elasticsearch.xpack.inference.external.http.sender.DocumentsOnlyInput;
Expand All @@ -44,7 +40,6 @@
import java.util.Map;
import java.util.Set;

import static org.elasticsearch.xpack.core.inference.results.ResultUtils.createInvalidChunkedResultException;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.createInvalidModelException;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.parsePersistedConfigErrorMsg;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.removeFromMapOrDefaultEmpty;
Expand Down Expand Up @@ -246,19 +241,6 @@ protected void doChunkedInfer(
}
}

private static List<ChunkedInferenceServiceResults> translateToChunkedResults(
List<String> inputs,
InferenceServiceResults inferenceResults
) {
if (inferenceResults instanceof InferenceTextEmbeddingFloatResults textEmbeddingResults) {
return InferenceChunkedTextEmbeddingFloatResults.listOf(inputs, textEmbeddingResults);
} else if (inferenceResults instanceof ErrorInferenceResults error) {
return List.of(new ErrorChunkedInferenceResults(error.getException()));
} else {
throw createInvalidChunkedResultException(InferenceTextEmbeddingFloatResults.NAME, inferenceResults.getWriteableName());
}
}

/**
* For text embedding models get the embedding size and
* update the service settings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import static org.elasticsearch.xpack.inference.services.ServiceUtils.extractOptionalPositiveInteger;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.extractRequiredString;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.extractSimilarity;
import static org.elasticsearch.xpack.inference.services.ServiceUtils.removeAsType;
import static org.elasticsearch.xpack.inference.services.mistral.MistralConstants.MODEL_FIELD;

public class MistralEmbeddingsServiceSettings extends FilteredXContentObject implements ServiceSettings {
Expand Down Expand Up @@ -67,7 +66,7 @@ public static MistralEmbeddingsServiceSettings fromMap(Map<String, Object> map,
MistralService.NAME,
context
);
Integer dims = removeAsType(map, DIMENSIONS, Integer.class);
Integer dims = extractOptionalPositiveInteger(map, DIMENSIONS, ModelConfigurations.SERVICE_SETTINGS, validationException);

if (validationException.validationErrors().isEmpty() == false) {
throw validationException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1048,13 +1048,18 @@ public void testChunkedInfer_CallsInfer_ConvertsFloatResponse_ForEmbeddings() th

try (var service = new AmazonBedrockService(factory, amazonBedrockFactory, createWithEmptySettings(threadPool))) {
try (var requestSender = (AmazonBedrockMockRequestSender) amazonBedrockFactory.createSender()) {
var mockResults = new InferenceTextEmbeddingFloatResults(
List.of(
new InferenceTextEmbeddingFloatResults.InferenceFloatEmbedding(new float[] { 0.123F, 0.678F }),
new InferenceTextEmbeddingFloatResults.InferenceFloatEmbedding(new float[] { 0.456F, 0.987F })
)
);
requestSender.enqueue(mockResults);
{
var mockResults1 = new InferenceTextEmbeddingFloatResults(
List.of(new InferenceTextEmbeddingFloatResults.InferenceFloatEmbedding(new float[] { 0.123F, 0.678F }))
);
requestSender.enqueue(mockResults1);
}
{
var mockResults2 = new InferenceTextEmbeddingFloatResults(
List.of(new InferenceTextEmbeddingFloatResults.InferenceFloatEmbedding(new float[] { 0.223F, 0.278F }))
);
requestSender.enqueue(mockResults2);
}

var model = AmazonBedrockEmbeddingsModelTests.createModel(
"id",
Expand Down Expand Up @@ -1089,7 +1094,7 @@ public void testChunkedInfer_CallsInfer_ConvertsFloatResponse_ForEmbeddings() th
var floatResult = (InferenceChunkedTextEmbeddingFloatResults) results.get(1);
assertThat(floatResult.chunks(), hasSize(1));
assertEquals("xyz", floatResult.chunks().get(0).matchedText());
assertArrayEquals(new float[] { 0.456F, 0.987F }, floatResult.chunks().get(0).embedding(), 0.0f);
assertArrayEquals(new float[] { 0.223F, 0.278F }, floatResult.chunks().get(0).embedding(), 0.0f);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package org.elasticsearch.xpack.inference.services.mistral.embeddings;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.io.stream.ByteArrayStreamInput;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.core.Nullable;
Expand All @@ -27,6 +28,7 @@
import java.util.Map;

import static org.elasticsearch.xpack.inference.services.ServiceFields.SIMILARITY;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.is;

public class MistralEmbeddingsServiceSettingsTests extends ESTestCase {
Expand Down Expand Up @@ -77,6 +79,84 @@ public void testFromMap_PersistentContext_DoesNotThrowException_WhenDimensionsIs
assertThat(serviceSettings, is(new MistralEmbeddingsServiceSettings(model, null, null, null, null)));
}

public void testFromMap_ThrowsException_WhenDimensionsAreZero() {
var model = "mistral-embed";
var dimensions = 0;

var settingsMap = createRequestSettingsMap(model, dimensions, null, SimilarityMeasure.COSINE);

var thrownException = expectThrows(
ValidationException.class,
() -> MistralEmbeddingsServiceSettings.fromMap(settingsMap, ConfigurationParseContext.REQUEST)
);

assertThat(
thrownException.getMessage(),
containsString("Validation Failed: 1: [service_settings] Invalid value [0]. [dimensions] must be a positive integer;")
);
}

public void testFromMap_ThrowsException_WhenDimensionsAreNegative() {
var model = "mistral-embed";
var dimensions = randomNegativeInt();

var settingsMap = createRequestSettingsMap(model, dimensions, null, SimilarityMeasure.COSINE);

var thrownException = expectThrows(
ValidationException.class,
() -> MistralEmbeddingsServiceSettings.fromMap(settingsMap, ConfigurationParseContext.REQUEST)
);

assertThat(
thrownException.getMessage(),
containsString(
Strings.format(
"Validation Failed: 1: [service_settings] Invalid value [%d]. [dimensions] must be a positive integer;",
dimensions
)
)
);
}

public void testFromMap_ThrowsException_WhenMaxInputTokensAreZero() {
var model = "mistral-embed";
var maxInputTokens = 0;

var settingsMap = createRequestSettingsMap(model, null, maxInputTokens, SimilarityMeasure.COSINE);

var thrownException = expectThrows(
ValidationException.class,
() -> MistralEmbeddingsServiceSettings.fromMap(settingsMap, ConfigurationParseContext.REQUEST)
);

assertThat(
thrownException.getMessage(),
containsString("Validation Failed: 1: [service_settings] Invalid value [0]. [max_input_tokens] must be a positive integer;")
);
}

public void testFromMap_ThrowsException_WhenMaxInputTokensAreNegative() {
var model = "mistral-embed";
var maxInputTokens = randomNegativeInt();

var settingsMap = createRequestSettingsMap(model, null, maxInputTokens, SimilarityMeasure.COSINE);

var thrownException = expectThrows(
ValidationException.class,
() -> MistralEmbeddingsServiceSettings.fromMap(settingsMap, ConfigurationParseContext.REQUEST)
);

assertThat(
thrownException.getMessage(),
containsString(
Strings.format(
"Validation Failed: 1: [service_settings] Invalid value [%d]. [max_input_tokens] must be a positive integer;",
maxInputTokens
)
)
);
}

public void testFromMap_PersistentContext_DoesNotThrowException_WhenSimilarityIsPresent() {
var model = "mistral-embed";

Expand Down

0 comments on commit 257028f

Please sign in to comment.