From 624a365a31722789fbe27917ccc67efc712f9f45 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Feb 2025 18:51:20 +0100 Subject: [PATCH 01/15] Logsdb and source only snapshots. Addresses a few issues with logsdb and source only snapshots: * Avoid initializing index sorting, because sort fields will not have doc values. * Also disable doc value skippers when doc values get disabled. * As part of source only validation figure out what the nested parent field is. * Avoid initializing _source.mode, because otherwise an empty _source:{} gets serialized in the restored mapping. Also added a few more tests that snapshot and restore logsdb data streams. --- .../elasticsearch/common/lucene/Lucene.java | 12 + .../org/elasticsearch/index/IndexService.java | 3 +- .../index/mapper/SourceFieldMapper.java | 3 +- .../sourceonly/SourceOnlySnapshot.java | 3 +- .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 341 ++++++++++++++++++ 5 files changed, 359 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 073000979918e..51819a7cb5a88 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -20,6 +20,8 @@ import org.apache.lucene.index.ConcurrentMergeScheduler; import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.FieldInfo; +import org.apache.lucene.index.FieldInfos; import org.apache.lucene.index.FilterCodecReader; import org.apache.lucene.index.FilterDirectoryReader; import org.apache.lucene.index.FilterLeafReader; @@ -190,7 +192,16 @@ public static SegmentInfos pruneUnreferencedFiles(String segmentsFileName, Direc throw new IllegalStateException("no commit found in the directory"); } } + String parentField = null; final IndexCommit cp = getIndexCommit(si, directory); + try (var reader = DirectoryReader.open(cp)) { + var topLevelFieldInfos = FieldInfos.getMergedFieldInfos(reader); + for (FieldInfo fieldInfo : topLevelFieldInfos) { + if (fieldInfo.isParentField()) { + parentField = fieldInfo.getName(); + } + } + } try ( IndexWriter writer = new IndexWriter( directory, @@ -198,6 +209,7 @@ public static SegmentInfos pruneUnreferencedFiles(String segmentsFileName, Direc .setIndexCommit(cp) .setCommitOnClose(false) .setOpenMode(IndexWriterConfig.OpenMode.APPEND) + .setParentField(parentField) ) ) { // do nothing and close this will kick off IndexFileDeleter which will remove all pending files diff --git a/server/src/main/java/org/elasticsearch/index/IndexService.java b/server/src/main/java/org/elasticsearch/index/IndexService.java index 5512dffdda53e..baba9e94db7a7 100644 --- a/server/src/main/java/org/elasticsearch/index/IndexService.java +++ b/server/src/main/java/org/elasticsearch/index/IndexService.java @@ -232,7 +232,8 @@ public IndexService( mapperMetrics ); this.indexFieldData = new IndexFieldDataService(indexSettings, indicesFieldDataCache, circuitBreakerService); - if (indexSettings.getIndexSortConfig().hasIndexSort()) { + boolean sourceOnly = Boolean.parseBoolean(indexSettings.getSettings().get("index.source_only")); + if (indexSettings.getIndexSortConfig().hasIndexSort() && sourceOnly == false) { // we delay the actual creation of the sort order for this index because the mapping has not been merged yet. // The sort order is validated right after the merge of the mapping later in the process. this.indexSortSupplier = () -> indexSettings.getIndexSortConfig() diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index 5ee7ee2ae9bf0..cf409645c7904 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -176,7 +176,8 @@ public Builder( true, () -> null, (n, c, o) -> Mode.valueOf(o.toString().toUpperCase(Locale.ROOT)), - m -> toType(m).enabled.explicit() ? null : toType(m).mode, + // Avoid initializing _source.mode if it doesn't need to be serialized: + m -> toType(m).enabled.explicit() ? null : toType(m).serializeMode ? toType(m).mode : null, (b, n, v) -> b.field(n, v.toString().toLowerCase(Locale.ROOT)), v -> v.toString().toLowerCase(Locale.ROOT) ).setMergeValidator((previous, current, conflicts) -> (previous == current) || current != Mode.STORED) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java index c76af6b0cfa09..731ab15001414 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/snapshots/sourceonly/SourceOnlySnapshot.java @@ -9,6 +9,7 @@ import org.apache.lucene.codecs.Codec; import org.apache.lucene.index.CheckIndex; import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.DocValuesSkipIndexType; import org.apache.lucene.index.DocValuesType; import org.apache.lucene.index.FieldInfo; import org.apache.lucene.index.FieldInfos; @@ -252,7 +253,7 @@ private SegmentCommitInfo syncSegment( false, IndexOptions.NONE, DocValuesType.NONE, - fieldInfo.docValuesSkipIndexType(), + DocValuesSkipIndexType.NONE, -1, fieldInfo.attributes(), 0, diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java new file mode 100644 index 0000000000000..5652c9e389857 --- /dev/null +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -0,0 +1,341 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.logsdb; + +import org.apache.http.client.methods.HttpPut; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.elasticsearch.common.network.InetAddresses; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.time.DateFormatter; +import org.elasticsearch.common.time.FormatNames; +import org.elasticsearch.repositories.fs.FsRepository; +import org.elasticsearch.test.cluster.ElasticsearchCluster; +import org.elasticsearch.test.cluster.local.distribution.DistributionType; +import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.test.rest.ObjectPath; +import org.junit.After; +import org.junit.ClassRule; +import org.junit.rules.RuleChain; +import org.junit.rules.TemporaryFolder; +import org.junit.rules.TestRule; + +import java.io.IOException; +import java.net.InetAddress; +import java.time.Instant; +import java.util.List; +import java.util.Locale; +import java.util.Map; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasEntry; +import static org.hamcrest.Matchers.hasSize; + +public class LogsdbSnapshotRestoreIT extends ESRestTestCase { + + private static TemporaryFolder repoDirectory = new TemporaryFolder(); + + private static ElasticsearchCluster cluster = ElasticsearchCluster.local() + .distribution(DistributionType.DEFAULT) + .setting("path.repo", () -> repoDirectory.getRoot().getPath()) + .setting("xpack.security.enabled", "false") + .setting("xpack.license.self_generated.type", "trial") + .build(); + + @ClassRule + public static TestRule ruleChain = RuleChain.outerRule(repoDirectory).around(cluster); + + static final String LOGS_TEMPLATE = """ + { + "index_patterns": [ "logs-*-*" ], + "data_stream": {}, + "priority": 1000, + "template": { + "settings": { + "index": { + "mapping": { + "source":{ + "mode": "{{source_mode}}" + } + } + } + }, + "mappings": { + "properties": { + "@timestamp" : { + "type": "date" + }, + "host.name": { + "type": "keyword" + }, + "pid": { + "type": "long" + }, + "method": { + "type": "keyword" + }, + "message": { + "type": "text" + }, + "ip_address": { + "type": "ip" + }, + "my_object_array": { + "type": "{{array_type}}" + } + } + } + } + }"""; + + static final String DOC_TEMPLATE = """ + { + "@timestamp": "%s", + "host.name": "%s", + "pid": "%d", + "method": "%s", + "message": "%s", + "ip_address": "%s", + "memory_usage_bytes": "%d", + "my_object_array": [ + { + "field_1": "a", + "field_2": "b" + }, + { + "field_1": "c", + "field_2": "d" + } + ] + } + """; + + @Override + protected String getTestRestCluster() { + return cluster.getHttpAddresses(); + } + + public void testSnapshotRestore() throws Exception { + snapshotAndRestore("synthetic", "object", false); + } + + public void testSnapshotRestoreWithSourceOnlyRepository() throws Exception { + snapshotAndFail("synthetic", "object", true); + } + + public void testSnapshotRestoreNested() throws Exception { + snapshotAndRestore("synthetic", "nested", false); + } + + public void testSnapshotRestoreNestedWithSourceOnlyRepository() throws Exception { + snapshotAndFail("synthetic", "nested", true); + } + + public void testSnapshotRestoreStoredSource() throws Exception { + snapshotAndRestore("stored", "object", false); + } + + public void testSnapshotRestoreStoredSourceWithSourceOnlyRepository() throws Exception { + snapshotAndRestore("stored", "object", true); + } + + public void testSnapshotRestoreStoredSourceNested() throws Exception { + snapshotAndRestore("stored", "nested", false); + } + + public void testSnapshotRestoreStoredSourceNestedWithSourceOnlyRepository() throws Exception { + snapshotAndRestore("stored", "nested", true); + } + + @After + public void cleanup() throws Exception { + deleteSnapshot("my-repository", "my-snapshot", true); + deleteRepository("my-repository"); + deleteDataStream("logs-my-test"); + } + + static void snapshotAndRestore(String sourceMode, String arrayType, boolean sourceOnly) throws IOException { + String dataStreamName = "logs-my-test"; + String repositoryName = "my-repository"; + if (sourceOnly) { + var repositorySettings = Settings.builder() + .put("delegate_type", "fs") + .put("location", repoDirectory.getRoot().getPath()) + .build(); + registerRepository(repositoryName, "source", true, repositorySettings); + } else { + var repositorySettings = Settings.builder().put("location", repoDirectory.getRoot().getPath()).build(); + registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); + } + + putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", sourceMode).replace("{{array_type}}", arrayType)); + for (int i = 0; i < 100; i++) { + indexDocument( + dataStreamName, + document( + Instant.now(), + randomAlphaOfLength(10), + randomNonNegativeLong(), + randomFrom("PUT", "POST", "GET"), + randomAlphaOfLength(32), + randomIp(randomBoolean()), + randomLongBetween(1_000_000L, 2_000_000L) + ) + ); + } + refresh(dataStreamName); + assertDocCount(client(), dataStreamName, 100); + assertDataStream(dataStreamName, sourceMode); + + String snapshotName = "my-snapshot"; + var snapshotResponse = performSnapshot(repositoryName, dataStreamName, snapshotName, true); + assertOK(snapshotResponse); + var snapshotResponseBody = entityAsMap(snapshotResponse); + Map snapshotItem = (Map) snapshotResponseBody.get("snapshot"); + List failures = (List) snapshotItem.get("failures"); + assertThat(failures, empty()); + deleteDataStream(dataStreamName); + assertDocCount(client(), dataStreamName, 0); + + restoreSnapshot(repositoryName, snapshotName, true); + assertDataStream(dataStreamName, sourceMode); + assertDocCount(client(), dataStreamName, 100); + } + + static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceOnly) throws IOException { + String dataStreamName = "logs-my-test"; + String repositoryName = "my-repository"; + if (sourceOnly) { + var repositorySettings = Settings.builder() + .put("delegate_type", "fs") + .put("location", repoDirectory.getRoot().getPath()) + .build(); + registerRepository(repositoryName, "source", true, repositorySettings); + } else { + var repositorySettings = Settings.builder().put("location", repoDirectory.getRoot().getPath()).build(); + registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); + } + + putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", sourceMode).replace("{{array_type}}", arrayType)); + for (int i = 0; i < 100; i++) { + indexDocument( + dataStreamName, + document( + Instant.now(), + randomAlphaOfLength(10), + randomNonNegativeLong(), + randomFrom("PUT", "POST", "GET"), + randomAlphaOfLength(32), + randomIp(randomBoolean()), + randomLongBetween(1_000_000L, 2_000_000L) + ) + ); + } + refresh(dataStreamName); + assertDocCount(client(), dataStreamName, 100); + assertDataStream(dataStreamName, sourceMode); + + String snapshotName = "my-snapshot"; + var snapshotResponse = performSnapshot(repositoryName, dataStreamName, snapshotName, true); + assertOK(snapshotResponse); + var snapshotResponseBody = entityAsMap(snapshotResponse); + Map snapshotItem = (Map) snapshotResponseBody.get("snapshot"); + List failures = (List) snapshotItem.get("failures"); + assertThat(failures, hasSize(1)); + Map failure = (Map) failures.get(0); + assertThat( + (String) failure.get("reason"), + containsString( + "Can't snapshot _source only on an index that has incomplete source ie. has _source disabled or filters the source" + ) + ); + } + + static void deleteDataStream(String dataStreamName) throws IOException { + assertOK(client().performRequest(new Request("DELETE", "/_data_stream/" + dataStreamName))); + } + + static void putTemplate(String templateName, String template) throws IOException { + final Request request = new Request("PUT", "/_index_template/" + templateName); + request.setJsonEntity(template); + assertOK(client().performRequest(request)); + } + + static void indexDocument(String indexOrtDataStream, String doc) throws IOException { + final Request request = new Request("POST", "/" + indexOrtDataStream + "/_doc?refresh=true"); + request.setJsonEntity(doc); + final Response response = client().performRequest(request); + assertOK(response); + assertThat(entityAsMap(response).get("result"), equalTo("created")); + } + + static String document( + final Instant timestamp, + final String hostname, + long pid, + final String method, + final String message, + final InetAddress ipAddress, + long memoryUsageBytes + ) { + return String.format( + Locale.ROOT, + DOC_TEMPLATE, + DateFormatter.forPattern(FormatNames.DATE_TIME.getName()).format(timestamp), + hostname, + pid, + method, + message, + InetAddresses.toAddrString(ipAddress), + memoryUsageBytes + ); + } + + static Response performSnapshot(String repository, String dataStreamName, String snapshot, boolean waitForCompletion) + throws IOException { + final Request request = new Request(HttpPut.METHOD_NAME, "_snapshot/" + repository + '/' + snapshot); + request.setJsonEntity(""" + { + "indices": "{{dataStreamName}}" + } + """.replace("{{dataStreamName}}", dataStreamName)); + request.addParameter("wait_for_completion", Boolean.toString(waitForCompletion)); + + return client().performRequest(request); + } + + static void assertDataStream(String dataStreamName, final String sourceMode) throws IOException { + String indexName = getWriteBackingIndex(dataStreamName, 0); + var flatSettings = (Map) ((Map) getIndexSettings(indexName).get(indexName)).get("settings"); + assertThat(flatSettings, hasEntry("index.mode", "logsdb")); + assertThat(flatSettings, hasEntry("index.mapping.source.mode", sourceMode)); + } + + static String getWriteBackingIndex(String dataStreamName, int backingIndex) throws IOException { + final Request request = new Request("GET", "_data_stream/" + dataStreamName); + final List dataStreams = (List) entityAsMap(client().performRequest(request)).get("data_streams"); + final Map dataStream = (Map) dataStreams.get(0); + final List backingIndices = (List) dataStream.get("indices"); + return (String) ((Map) backingIndices.get(backingIndex)).get("index_name"); + } + + public static void assertDocCount(RestClient client, String indexName, long docCount) throws IOException { + Request countReq = new Request("GET", "/" + indexName + "/_count"); + countReq.addParameter("ignore_unavailable", "true"); + ObjectPath resp = ObjectPath.createFromResponse(client.performRequest(countReq)); + assertEquals( + "expected " + docCount + " documents but it was a different number", + docCount, + Long.parseLong(resp.evaluate("count").toString()) + ); + } + +} From 04c4bb15cab2c99b8a33c5867f569998cf53671e Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Feb 2025 19:38:24 +0100 Subject: [PATCH 02/15] suppress forbidden api usage --- .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index 5652c9e389857..3f4a0400d574d 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -15,6 +15,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.FormatNames; +import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; @@ -45,7 +46,7 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase { private static ElasticsearchCluster cluster = ElasticsearchCluster.local() .distribution(DistributionType.DEFAULT) - .setting("path.repo", () -> repoDirectory.getRoot().getPath()) + .setting("path.repo", () -> getRepoPath()) .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") .build(); @@ -162,17 +163,15 @@ public void cleanup() throws Exception { deleteDataStream("logs-my-test"); } + @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File") static void snapshotAndRestore(String sourceMode, String arrayType, boolean sourceOnly) throws IOException { String dataStreamName = "logs-my-test"; String repositoryName = "my-repository"; if (sourceOnly) { - var repositorySettings = Settings.builder() - .put("delegate_type", "fs") - .put("location", repoDirectory.getRoot().getPath()) - .build(); + var repositorySettings = Settings.builder().put("delegate_type", "fs").put("location", getRepoPath()).build(); registerRepository(repositoryName, "source", true, repositorySettings); } else { - var repositorySettings = Settings.builder().put("location", repoDirectory.getRoot().getPath()).build(); + var repositorySettings = Settings.builder().put("location", getRepoPath()).build(); registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); } @@ -214,13 +213,10 @@ static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceO String dataStreamName = "logs-my-test"; String repositoryName = "my-repository"; if (sourceOnly) { - var repositorySettings = Settings.builder() - .put("delegate_type", "fs") - .put("location", repoDirectory.getRoot().getPath()) - .build(); + var repositorySettings = Settings.builder().put("delegate_type", "fs").put("location", getRepoPath()).build(); registerRepository(repositoryName, "source", true, repositorySettings); } else { - var repositorySettings = Settings.builder().put("location", repoDirectory.getRoot().getPath()).build(); + var repositorySettings = Settings.builder().put("location", getRepoPath()).build(); registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); } @@ -338,4 +334,9 @@ public static void assertDocCount(RestClient client, String indexName, long docC ); } + @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File") + private static String getRepoPath() { + return repoDirectory.getRoot().getPath(); + } + } From 385e8214e12144d7a0071fb703095ed975e96a6d Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Feb 2025 19:13:15 +0100 Subject: [PATCH 03/15] Update docs/changelog/122199.yaml --- docs/changelog/122199.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/122199.yaml diff --git a/docs/changelog/122199.yaml b/docs/changelog/122199.yaml new file mode 100644 index 0000000000000..d197d83881998 --- /dev/null +++ b/docs/changelog/122199.yaml @@ -0,0 +1,5 @@ +pr: 122199 +summary: Logsdb and source only snapshots +area: Logs +type: bug +issues: [] From 62334b308247230e3898f89d19e10ad0122baa3b Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Feb 2025 19:56:23 +0100 Subject: [PATCH 04/15] removed unneeded annotation --- .../org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index 3f4a0400d574d..f16d7a0f6ffb5 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -163,7 +163,6 @@ public void cleanup() throws Exception { deleteDataStream("logs-my-test"); } - @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File") static void snapshotAndRestore(String sourceMode, String arrayType, boolean sourceOnly) throws IOException { String dataStreamName = "logs-my-test"; String repositoryName = "my-repository"; From 6730822ecb24d4398f15e039c44890fdde072136 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 10 Feb 2025 19:59:31 +0100 Subject: [PATCH 05/15] Now that SFM doesn't get serialized the parser needs trigger validation based on default instances being returned. --- .../elasticsearch/index/mapper/SourceFieldMapper.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index cf409645c7904..56f252c1b10e0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -301,10 +301,11 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) { if (indexMode == IndexMode.STANDARD && settingSourceMode == Mode.STORED) { return DEFAULT; } + SourceFieldMapper sourceFieldMapper = null; if (onOrAfterDeprecateModeVersion(c.indexVersionCreated())) { - return resolveStaticInstance(settingSourceMode); + sourceFieldMapper = resolveStaticInstance(settingSourceMode); } else { - return new SourceFieldMapper( + sourceFieldMapper = new SourceFieldMapper( settingSourceMode, Explicit.IMPLICIT_TRUE, Strings.EMPTY_ARRAY, @@ -313,6 +314,10 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) { c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_MODE_ATTRIBUTE_NOOP) ); } + // By default no attributes are specified and so the Builder doesn't get used. + // Need to validate the returned instance based on index mode: + indexMode.validateSourceFieldMapper(sourceFieldMapper); + return sourceFieldMapper; }, c -> new Builder( c.getIndexSettings().getMode(), From cd9beedbd2a7ac18b0384080b2d61abc4983b1b7 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 11 Feb 2025 09:13:46 +0100 Subject: [PATCH 06/15] iter tests --- .../elasticsearch/index/mapper/SourceFieldMapperTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java index 70010084cdb96..3840f45465e4d 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java @@ -267,14 +267,14 @@ public void testSyntheticSourceInTimeSeries() throws IOException { }); DocumentMapper mapper = createTimeSeriesModeDocumentMapper(mapping); assertTrue(mapper.sourceMapper().isSynthetic()); - assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString()); + assertEquals("{}", mapper.sourceMapper().toString()); } public void testSyntheticSourceWithLogsIndexMode() throws IOException { XContentBuilder mapping = fieldMapping(b -> { b.field("type", "keyword"); }); DocumentMapper mapper = createLogsModeDocumentMapper(mapping); assertTrue(mapper.sourceMapper().isSynthetic()); - assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString()); + assertEquals("{}", mapper.sourceMapper().toString()); } public void testSupportsNonDefaultParameterValues() throws IOException { From f3c6a9de9e15fef12aa62f89f66f1c05f18a60b1 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 11 Feb 2025 11:17:54 +0100 Subject: [PATCH 07/15] update mapping asserts to account for empty _source:{} in bwc cases --- .../index/mapper/DocumentMapper.java | 16 +++++++++++++++- .../index/mapper/MapperService.java | 3 ++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index 0d488e47c2e4f..ae20dbdacb049 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -68,7 +68,7 @@ public static DocumentMapper createEmpty(MapperService mapperService) { this.logger = Loggers.getLogger(getClass(), indexName); this.indexName = indexName; - assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version) + assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version) || equalsWithEmptySource(source) : "provided source [" + source + "] differs from mapping [" + mapping.toCompressedXContent() + "]"; } @@ -91,6 +91,20 @@ boolean isSyntheticSourceMalformed(CompressedXContent source, IndexVersion versi && version.onOrBefore(IndexVersions.V_8_10_0); } + /** + * In case when source is empty in bwc situations. Empty _source now doesn't get serialized anymore. + */ + boolean equalsWithEmptySource(CompressedXContent source) { + String sourceAsString = source.string(); + if (sourceAsString.contains(",\"_source\":{}")) { + return mapping().toString().equals(sourceAsString.replace(",\"_source\":{}", "")); + } else if (sourceAsString.contains("\"_source\":{}")) { + return mapping().toString().equals(sourceAsString.replace("\"_source\":{}", "")); + } else { + return false; + } + } + public Mapping mapping() { return mappingLookup.getMapping(); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 7958fd8e51525..6b142c799ceb2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -411,7 +411,8 @@ boolean assertNoUpdateRequired(final IndexMetadata newIndexMetadata) { final CompressedXContent currentSource = this.mapper.mappingSource(); final CompressedXContent newSource = newMapping.toCompressedXContent(); if (Objects.equals(currentSource, newSource) == false - && mapper.isSyntheticSourceMalformed(currentSource, indexVersionCreated) == false) { + && mapper.isSyntheticSourceMalformed(currentSource, indexVersionCreated) == false + && mapper.equalsWithEmptySource(currentSource) == false) { throw new IllegalStateException( "expected current mapping [" + currentSource + "] to be the same as new mapping [" + newSource + "]" ); From d4777d54f74f3a4091cdf075effc92ed7eaf7ef5 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 11 Feb 2025 12:58:15 +0100 Subject: [PATCH 08/15] disable assertion jvm arg work around --- qa/mixed-cluster/build.gradle | 3 +++ .../xpack/downsample/MixedClusterDownsampleRestIT.java | 3 +++ .../java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java | 3 +++ 3 files changed, 9 insertions(+) diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle index 0889837457285..9b6a17e567fd9 100644 --- a/qa/mixed-cluster/build.gradle +++ b/qa/mixed-cluster/build.gradle @@ -90,6 +90,9 @@ buildParams.bwcVersions.withWireCompatible { bwcVersion, baseName -> if (bwcVersion.onOrAfter(Version.fromString("8.4.0"))) { setting 'health.master_history.no_master_transitions_threshold', '10' } + // TODO: conditionally add jvmarg based on old version + // Avoid tripping assertion on old nodes: + jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper' requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") } diff --git a/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java b/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java index a4765271e7300..330da5cdb2ed5 100644 --- a/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java +++ b/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java @@ -25,6 +25,9 @@ public class MixedClusterDownsampleRestIT extends ESClientYamlSuiteTestCase { .withNode(node -> node.version(Version.CURRENT)) .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") + // TODO: conditionally add jvmarg based on old version + // Avoid tripping assertion on old nodes: + .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper") .build(); static Version getOldVersion() { diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java index 8a55624ed3a6e..605dc9f43cea1 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java @@ -23,6 +23,9 @@ public static ElasticsearchCluster mixedVersionCluster() { .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") .setting("cluster.routing.rebalance.enable", "none") // disable relocation until we have retry in ESQL + // TODO: conditionally add jvmarg based on old version + // Avoid tripping assertion on old nodes: + .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper") .build(); } } From 43f9d5dfe3dfe53aebec75e54f09e82293ebf3c4 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 11 Feb 2025 16:43:39 +0100 Subject: [PATCH 09/15] Revert changes to SFM's mode attribute initializer and instead disable for assertions for LogsdbSnapshotRestoreIT now. --- qa/mixed-cluster/build.gradle | 3 --- .../index/mapper/DocumentMapper.java | 16 +--------------- .../index/mapper/MapperService.java | 3 +-- .../index/mapper/SourceFieldMapper.java | 11 +++-------- .../index/mapper/SourceFieldMapperTests.java | 4 ++-- .../downsample/MixedClusterDownsampleRestIT.java | 3 --- .../xpack/esql/qa/mixed/Clusters.java | 3 --- .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 4 ++++ 8 files changed, 11 insertions(+), 36 deletions(-) diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle index 9b6a17e567fd9..0889837457285 100644 --- a/qa/mixed-cluster/build.gradle +++ b/qa/mixed-cluster/build.gradle @@ -90,9 +90,6 @@ buildParams.bwcVersions.withWireCompatible { bwcVersion, baseName -> if (bwcVersion.onOrAfter(Version.fromString("8.4.0"))) { setting 'health.master_history.no_master_transitions_threshold', '10' } - // TODO: conditionally add jvmarg based on old version - // Avoid tripping assertion on old nodes: - jvmArgs '-da:org.elasticsearch.index.mapper.DocumentMapper' requiresFeature 'es.index_mode_feature_flag_registered', Version.fromString("8.0.0") } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java index ae20dbdacb049..0d488e47c2e4f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentMapper.java @@ -68,7 +68,7 @@ public static DocumentMapper createEmpty(MapperService mapperService) { this.logger = Loggers.getLogger(getClass(), indexName); this.indexName = indexName; - assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version) || equalsWithEmptySource(source) + assert mapping.toCompressedXContent().equals(source) || isSyntheticSourceMalformed(source, version) : "provided source [" + source + "] differs from mapping [" + mapping.toCompressedXContent() + "]"; } @@ -91,20 +91,6 @@ boolean isSyntheticSourceMalformed(CompressedXContent source, IndexVersion versi && version.onOrBefore(IndexVersions.V_8_10_0); } - /** - * In case when source is empty in bwc situations. Empty _source now doesn't get serialized anymore. - */ - boolean equalsWithEmptySource(CompressedXContent source) { - String sourceAsString = source.string(); - if (sourceAsString.contains(",\"_source\":{}")) { - return mapping().toString().equals(sourceAsString.replace(",\"_source\":{}", "")); - } else if (sourceAsString.contains("\"_source\":{}")) { - return mapping().toString().equals(sourceAsString.replace("\"_source\":{}", "")); - } else { - return false; - } - } - public Mapping mapping() { return mappingLookup.getMapping(); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 6b142c799ceb2..7958fd8e51525 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -411,8 +411,7 @@ boolean assertNoUpdateRequired(final IndexMetadata newIndexMetadata) { final CompressedXContent currentSource = this.mapper.mappingSource(); final CompressedXContent newSource = newMapping.toCompressedXContent(); if (Objects.equals(currentSource, newSource) == false - && mapper.isSyntheticSourceMalformed(currentSource, indexVersionCreated) == false - && mapper.equalsWithEmptySource(currentSource) == false) { + && mapper.isSyntheticSourceMalformed(currentSource, indexVersionCreated) == false) { throw new IllegalStateException( "expected current mapping [" + currentSource + "] to be the same as new mapping [" + newSource + "]" ); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index 56f252c1b10e0..cb4e80c08eeb7 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -177,7 +177,7 @@ public Builder( () -> null, (n, c, o) -> Mode.valueOf(o.toString().toUpperCase(Locale.ROOT)), // Avoid initializing _source.mode if it doesn't need to be serialized: - m -> toType(m).enabled.explicit() ? null : toType(m).serializeMode ? toType(m).mode : null, + m -> toType(m).enabled.explicit() ? null : toType(m).mode, (b, n, v) -> b.field(n, v.toString().toLowerCase(Locale.ROOT)), v -> v.toString().toLowerCase(Locale.ROOT) ).setMergeValidator((previous, current, conflicts) -> (previous == current) || current != Mode.STORED) @@ -301,11 +301,10 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) { if (indexMode == IndexMode.STANDARD && settingSourceMode == Mode.STORED) { return DEFAULT; } - SourceFieldMapper sourceFieldMapper = null; if (onOrAfterDeprecateModeVersion(c.indexVersionCreated())) { - sourceFieldMapper = resolveStaticInstance(settingSourceMode); + return resolveStaticInstance(settingSourceMode); } else { - sourceFieldMapper = new SourceFieldMapper( + return new SourceFieldMapper( settingSourceMode, Explicit.IMPLICIT_TRUE, Strings.EMPTY_ARRAY, @@ -314,10 +313,6 @@ private static SourceFieldMapper resolveStaticInstance(final Mode sourceMode) { c.indexVersionCreated().onOrAfter(IndexVersions.SOURCE_MAPPER_MODE_ATTRIBUTE_NOOP) ); } - // By default no attributes are specified and so the Builder doesn't get used. - // Need to validate the returned instance based on index mode: - indexMode.validateSourceFieldMapper(sourceFieldMapper); - return sourceFieldMapper; }, c -> new Builder( c.getIndexSettings().getMode(), diff --git a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java index 3840f45465e4d..70010084cdb96 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/SourceFieldMapperTests.java @@ -267,14 +267,14 @@ public void testSyntheticSourceInTimeSeries() throws IOException { }); DocumentMapper mapper = createTimeSeriesModeDocumentMapper(mapping); assertTrue(mapper.sourceMapper().isSynthetic()); - assertEquals("{}", mapper.sourceMapper().toString()); + assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString()); } public void testSyntheticSourceWithLogsIndexMode() throws IOException { XContentBuilder mapping = fieldMapping(b -> { b.field("type", "keyword"); }); DocumentMapper mapper = createLogsModeDocumentMapper(mapping); assertTrue(mapper.sourceMapper().isSynthetic()); - assertEquals("{}", mapper.sourceMapper().toString()); + assertEquals("{\"_source\":{}}", mapper.sourceMapper().toString()); } public void testSupportsNonDefaultParameterValues() throws IOException { diff --git a/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java b/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java index 330da5cdb2ed5..a4765271e7300 100644 --- a/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java +++ b/x-pack/plugin/downsample/qa/mixed-cluster/src/yamlRestTest/java/org/elasticsearch/xpack/downsample/MixedClusterDownsampleRestIT.java @@ -25,9 +25,6 @@ public class MixedClusterDownsampleRestIT extends ESClientYamlSuiteTestCase { .withNode(node -> node.version(Version.CURRENT)) .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") - // TODO: conditionally add jvmarg based on old version - // Avoid tripping assertion on old nodes: - .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper") .build(); static Version getOldVersion() { diff --git a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java index 605dc9f43cea1..8a55624ed3a6e 100644 --- a/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java +++ b/x-pack/plugin/esql/qa/server/mixed-cluster/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/mixed/Clusters.java @@ -23,9 +23,6 @@ public static ElasticsearchCluster mixedVersionCluster() { .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") .setting("cluster.routing.rebalance.enable", "none") // disable relocation until we have retry in ESQL - // TODO: conditionally add jvmarg based on old version - // Avoid tripping assertion on old nodes: - .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper") .build(); } } diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index f16d7a0f6ffb5..94f3138b5e933 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -49,6 +49,10 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase { .setting("path.repo", () -> getRepoPath()) .setting("xpack.security.enabled", "false") .setting("xpack.license.self_generated.type", "trial") + // TODO: remove when initializing / serializing default SourceFieldMapper instance have been fixed: + // (SFM's mode attribute often gets initialized, even when mode attribute isn't set) + .jvmArg("-da:org.elasticsearch.index.mapper.DocumentMapper") + .jvmArg("-da:org.elasticsearch.index.mapper.MapperService") .build(); @ClassRule From e2addaf3bf32fe6753bd15f52762e185355b29ea Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 12 Feb 2025 09:08:16 +0100 Subject: [PATCH 10/15] added comment --- .../src/main/java/org/elasticsearch/common/lucene/Lucene.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java index 51819a7cb5a88..2aa87d808fc93 100644 --- a/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java +++ b/server/src/main/java/org/elasticsearch/common/lucene/Lucene.java @@ -192,6 +192,8 @@ public static SegmentInfos pruneUnreferencedFiles(String segmentsFileName, Direc throw new IllegalStateException("no commit found in the directory"); } } + // Need to figure out what the parent field is that, so that validation in IndexWriter doesn't fail + // if no parent field is configured, but FieldInfo says there is a parent field. String parentField = null; final IndexCommit cp = getIndexCommit(si, directory); try (var reader = DirectoryReader.open(cp)) { From 29df0bc5b80accd6c6aa9a35e28f715bccb18768 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 12 Feb 2025 09:17:41 +0100 Subject: [PATCH 11/15] hard code parameters --- .../index/mapper/SourceFieldMapper.java | 1 - .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java index cb4e80c08eeb7..5ee7ee2ae9bf0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java @@ -176,7 +176,6 @@ public Builder( true, () -> null, (n, c, o) -> Mode.valueOf(o.toString().toUpperCase(Locale.ROOT)), - // Avoid initializing _source.mode if it doesn't need to be serialized: m -> toType(m).enabled.explicit() ? null : toType(m).mode, (b, n, v) -> b.field(n, v.toString().toLowerCase(Locale.ROOT)), v -> v.toString().toLowerCase(Locale.ROOT) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index 94f3138b5e933..09ab2aecff241 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -133,7 +133,7 @@ public void testSnapshotRestore() throws Exception { } public void testSnapshotRestoreWithSourceOnlyRepository() throws Exception { - snapshotAndFail("synthetic", "object", true); + snapshotAndFail("object"); } public void testSnapshotRestoreNested() throws Exception { @@ -141,7 +141,7 @@ public void testSnapshotRestoreNested() throws Exception { } public void testSnapshotRestoreNestedWithSourceOnlyRepository() throws Exception { - snapshotAndFail("synthetic", "nested", true); + snapshotAndFail("nested"); } public void testSnapshotRestoreStoredSource() throws Exception { @@ -212,10 +212,10 @@ static void snapshotAndRestore(String sourceMode, String arrayType, boolean sour assertDocCount(client(), dataStreamName, 100); } - static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceOnly) throws IOException { + static void snapshotAndFail(String arrayType) throws IOException { String dataStreamName = "logs-my-test"; String repositoryName = "my-repository"; - if (sourceOnly) { + if (true) { var repositorySettings = Settings.builder().put("delegate_type", "fs").put("location", getRepoPath()).build(); registerRepository(repositoryName, "source", true, repositorySettings); } else { @@ -223,7 +223,7 @@ static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceO registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); } - putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", sourceMode).replace("{{array_type}}", arrayType)); + putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", "synthetic").replace("{{array_type}}", arrayType)); for (int i = 0; i < 100; i++) { indexDocument( dataStreamName, @@ -240,7 +240,7 @@ static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceO } refresh(dataStreamName); assertDocCount(client(), dataStreamName, 100); - assertDataStream(dataStreamName, sourceMode); + assertDataStream(dataStreamName, "synthetic"); String snapshotName = "my-snapshot"; var snapshotResponse = performSnapshot(repositoryName, dataStreamName, snapshotName, true); From 371229ae8e85527c1bde4055557221879fba4789 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 12 Feb 2025 10:52:25 +0100 Subject: [PATCH 12/15] applied test feedback --- .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 76 +++++++++++++------ 1 file changed, 54 insertions(+), 22 deletions(-) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index 09ab2aecff241..61ae5b181e987 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -10,17 +10,18 @@ import org.apache.http.client.methods.HttpPut; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; -import org.elasticsearch.client.RestClient; import org.elasticsearch.common.network.InetAddresses; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.time.FormatNames; +import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.core.SuppressForbidden; import org.elasticsearch.repositories.fs.FsRepository; import org.elasticsearch.test.cluster.ElasticsearchCluster; import org.elasticsearch.test.cluster.local.distribution.DistributionType; import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.test.rest.ObjectPath; +import org.elasticsearch.xcontent.XContentType; import org.junit.After; import org.junit.ClassRule; import org.junit.rules.RuleChain; @@ -34,6 +35,8 @@ import java.util.Locale; import java.util.Map; +import static org.elasticsearch.test.MapMatcher.assertMap; +import static org.elasticsearch.test.MapMatcher.matchesMap; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; @@ -78,11 +81,15 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase { "@timestamp" : { "type": "date" }, - "host.name": { - "type": "keyword" + "host": { + "properties": { + "name": { + "type": "keyword" + } + } }, "pid": { - "type": "long" + "type": "integer" }, "method": { "type": "keyword" @@ -104,8 +111,8 @@ public class LogsdbSnapshotRestoreIT extends ESRestTestCase { static final String DOC_TEMPLATE = """ { "@timestamp": "%s", - "host.name": "%s", - "pid": "%d", + "host": { "name": "%s"}, + "pid": %d, "method": "%s", "message": "%s", "ip_address": "%s", @@ -179,22 +186,22 @@ static void snapshotAndRestore(String sourceMode, String arrayType, boolean sour } putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", sourceMode).replace("{{array_type}}", arrayType)); + String[] docs = new String[100]; for (int i = 0; i < 100; i++) { - indexDocument( - dataStreamName, - document( - Instant.now(), - randomAlphaOfLength(10), - randomNonNegativeLong(), - randomFrom("PUT", "POST", "GET"), - randomAlphaOfLength(32), - randomIp(randomBoolean()), - randomLongBetween(1_000_000L, 2_000_000L) - ) + docs[i] = document( + Instant.now(), + String.format(Locale.ROOT, "host-%03d", i), + randomNonNegativeInt(), + randomFrom("PUT", "POST", "GET"), + randomAlphaOfLength(32), + randomIp(randomBoolean()), + randomLongBetween(1_000_000L, 2_000_000L) ); + indexDocument(dataStreamName, docs[i]); } refresh(dataStreamName); assertDocCount(client(), dataStreamName, 100); + assertSource(dataStreamName, docs); assertDataStream(dataStreamName, sourceMode); String snapshotName = "my-snapshot"; @@ -205,11 +212,12 @@ static void snapshotAndRestore(String sourceMode, String arrayType, boolean sour List failures = (List) snapshotItem.get("failures"); assertThat(failures, empty()); deleteDataStream(dataStreamName); - assertDocCount(client(), dataStreamName, 0); + assertDocCount(dataStreamName, 0); restoreSnapshot(repositoryName, snapshotName, true); assertDataStream(dataStreamName, sourceMode); - assertDocCount(client(), dataStreamName, 100); + assertDocCount(dataStreamName, 100); + assertSource(dataStreamName, docs); } static void snapshotAndFail(String arrayType) throws IOException { @@ -234,7 +242,7 @@ static void snapshotAndFail(String arrayType) throws IOException { randomFrom("PUT", "POST", "GET"), randomAlphaOfLength(32), randomIp(randomBoolean()), - randomLongBetween(1_000_000L, 2_000_000L) + randomIntBetween(1_000_000, 2_000_000) ) ); } @@ -326,10 +334,10 @@ static String getWriteBackingIndex(String dataStreamName, int backingIndex) thro return (String) ((Map) backingIndices.get(backingIndex)).get("index_name"); } - public static void assertDocCount(RestClient client, String indexName, long docCount) throws IOException { + static void assertDocCount(String indexName, long docCount) throws IOException { Request countReq = new Request("GET", "/" + indexName + "/_count"); countReq.addParameter("ignore_unavailable", "true"); - ObjectPath resp = ObjectPath.createFromResponse(client.performRequest(countReq)); + ObjectPath resp = ObjectPath.createFromResponse(client().performRequest(countReq)); assertEquals( "expected " + docCount + " documents but it was a different number", docCount, @@ -337,6 +345,30 @@ public static void assertDocCount(RestClient client, String indexName, long docC ); } + static void assertSource(String indexName, String[] docs) throws IOException { + Request searchReq = new Request("GET", "/" + indexName + "/_search"); + searchReq.addParameter("size", String.valueOf(docs.length)); + var response = client().performRequest(searchReq); + assertOK(response); + var responseBody = entityAsMap(response); + List hits = (List) ((Map) responseBody.get("hits")).get("hits"); + assertThat(hits, hasSize(docs.length)); + for (Object hit : hits) { + Map actualSource = (Map) ((Map) hit).get("_source"); + String actualHost = (String) ((Map) actualSource.get("host")).get("name"); + Map expectedSource = null; + for (String doc : docs) { + expectedSource = XContentHelper.convertToMap(XContentType.JSON.xContent(), doc, false); + String expectedHost = (String) ((Map) expectedSource.get("host")).get("name"); + if (expectedHost.equals(actualHost)) { + break; + } + } + + assertMap(actualSource, matchesMap(expectedSource)); + } + } + @SuppressForbidden(reason = "TemporaryFolder only has io.File methods, not nio.File") private static String getRepoPath() { return repoDirectory.getRoot().getPath(); From d1019f8c1f62929df5d29f9742d06815ebc2f04b Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 12 Feb 2025 09:59:59 +0000 Subject: [PATCH 13/15] [CI] Auto commit changes from spotless --- .../org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index 61ae5b181e987..e7e4f02880499 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -358,7 +358,7 @@ static void assertSource(String indexName, String[] docs) throws IOException { String actualHost = (String) ((Map) actualSource.get("host")).get("name"); Map expectedSource = null; for (String doc : docs) { - expectedSource = XContentHelper.convertToMap(XContentType.JSON.xContent(), doc, false); + expectedSource = XContentHelper.convertToMap(XContentType.JSON.xContent(), doc, false); String expectedHost = (String) ((Map) expectedSource.get("host")).get("name"); if (expectedHost.equals(actualHost)) { break; From 09e39883848585188cf2da59fee3b54b4db3bc62 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 12 Feb 2025 11:07:06 +0100 Subject: [PATCH 14/15] iter --- docs/changelog/122199.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/122199.yaml b/docs/changelog/122199.yaml index d197d83881998..172ae900bdabb 100644 --- a/docs/changelog/122199.yaml +++ b/docs/changelog/122199.yaml @@ -1,5 +1,5 @@ pr: 122199 -summary: Logsdb and source only snapshots +summary: Fix issues that prevents using search only snapshots for indices that use index sorting. This is includes Logsdb and time series indices. area: Logs type: bug issues: [] From 44ceea7407aae18a2b7d18f82bb24144a8541ac0 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 12 Feb 2025 11:10:20 +0100 Subject: [PATCH 15/15] remove redundant test code --- .../xpack/logsdb/LogsdbSnapshotRestoreIT.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java index e7e4f02880499..0b57d0ed8c4f0 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java @@ -223,13 +223,8 @@ static void snapshotAndRestore(String sourceMode, String arrayType, boolean sour static void snapshotAndFail(String arrayType) throws IOException { String dataStreamName = "logs-my-test"; String repositoryName = "my-repository"; - if (true) { - var repositorySettings = Settings.builder().put("delegate_type", "fs").put("location", getRepoPath()).build(); - registerRepository(repositoryName, "source", true, repositorySettings); - } else { - var repositorySettings = Settings.builder().put("location", getRepoPath()).build(); - registerRepository(repositoryName, FsRepository.TYPE, true, repositorySettings); - } + var repositorySettings = Settings.builder().put("delegate_type", "fs").put("location", getRepoPath()).build(); + registerRepository(repositoryName, "source", true, repositorySettings); putTemplate("my-template", LOGS_TEMPLATE.replace("{{source_mode}}", "synthetic").replace("{{array_type}}", arrayType)); for (int i = 0; i < 100; i++) {