Logsdb and source only snapshots. #122199

martijnvg · 2025-02-10T18:12:52Z

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.

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.

elasticsearchmachine · 2025-02-10T18:13:15Z

Pinging @elastic/es-storage-engine (Team:StorageEngine)

elasticsearchmachine · 2025-02-10T18:13:17Z

Hi @martijnvg, I've created a changelog YAML for you.

…on based on default instances being returned.

kkrik-es · 2025-02-11T09:24:23Z

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,


Nit: let's add parentheses to denote the sequence of the conditional branches.

server/src/main/java/org/elasticsearch/common/lucene/Lucene.java

kkrik-es · 2025-02-11T09:31:07Z

...gin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java

+        assertDocCount(client(), dataStreamName, 100);
+    }
+
+    static void snapshotAndFail(String sourceMode, String arrayType, boolean sourceOnly) throws IOException {


Params need to be "synthetic" and false, for this to fail?

Yes, the issues are with sourceOnly snapshots.

Let's then hard-code these and remove them from method args.

kkrik-es · 2025-02-11T09:34:05Z

...gin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java

+
+        restoreSnapshot(repositoryName, snapshotName, true);
+        assertDataStream(dataStreamName, sourceMode);
+        assertDocCount(client(), dataStreamName, 100);


Shall we also compare the source for the two old and the new index?

kkrik-es

Nice, a few minor comments and questions.

…e for assertions for LogsdbSnapshotRestoreIT now.

kkrik-es · 2025-02-12T09:57:14Z

...gin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java

        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 {


Nit: remove, redundant.

martijnvg · 2025-02-12T10:03:10Z

...gin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/LogsdbSnapshotRestoreIT.java

+        .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")


This is an issue that we need to address. We always serialize an empty _source, because the mode attribute's initializer initializes the mode even if we don't need to include/serialize the mode attribute. Changing that is tricky and we need to take bwc into account, because DocumentMapper's assertion will trip on older nodes.

The reason why we need to disable DocumentMapper's and MapperService's assertions, is because the mapping update that happens when source only snapshots (in SourceOnlySnapshotRepository#metadataToSnapshot(...)). Which is metadata only mapping update that triggers the mentioned assertions to trip. This isn't an issue in production, but should fixed.

I will work on a follow up pr that avoids serializing an empty source if _source defaults are active. That fixes the assertion from tripping (like the initial commit did in this PR). But given the trickiness is better to do in a followup pr.

Backporting elastic#122199 to 8.x branch. 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. Also added a few more tests that snapshot and restore logsdb data streams.

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. Also added a few more tests that snapshot and restore logsdb data streams.

Backporting #122199 to 9.0 branch. 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. Also added a few more tests that snapshot and restore logsdb data streams.

* [8.x] Logsdb and source only snapshots. Backporting #122199 to 8.x branch. 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. Also added a few more tests that snapshot and restore logsdb data streams. * fix test

* [8.x] Logsdb and source only snapshots. Backporting elastic#122199 to 8.x branch. 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. Also added a few more tests that snapshot and restore logsdb data streams. * fix test

* [8.x] Logsdb and source only snapshots. Backporting #122199 to 8.x branch. 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. Also added a few more tests that snapshot and restore logsdb data streams. * fix test

martijnvg added >bug :StorageEngine/Logs You know, for Logs v9.0.0 v8.17.1 v8.18.1 v8.19.0 v9.1.0 labels Feb 10, 2025

elasticsearchmachine added the Team:StorageEngine label Feb 10, 2025

martijnvg added 3 commits February 10, 2025 19:38

suppress forbidden api usage

04c4bb1

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

a06803a

Update docs/changelog/122199.yaml

385e821

martijnvg force-pushed the logsdb_search_only_snapshots branch from 9bd067a to 385e821 Compare February 10, 2025 18:39

martijnvg added 4 commits February 10, 2025 19:56

removed unneeded annotation

62334b3

Now that SFM doesn't get serialized the parser needs trigger validati…

6730822

…on based on default instances being returned.

iter tests

cd9beed

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

e7c36da

kkrik-es reviewed Feb 11, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/common/lucene/Lucene.java Show resolved Hide resolved

kkrik-es reviewed Feb 11, 2025

View reviewed changes

martijnvg added 6 commits February 11, 2025 11:17

update mapping asserts to account for empty _source:{} in bwc cases

f3c6a9d

disable assertion jvm arg work around

d4777d5

Revert changes to SFM's mode attribute initializer and instead disabl…

43f9d5d

…e for assertions for LogsdbSnapshotRestoreIT now.

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

f4bf08b

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

0e3d2f7

added comment

e2addaf

martijnvg added 2 commits February 12, 2025 09:17

hard code parameters

29df0bc

applied test feedback

371229a

kkrik-es reviewed Feb 12, 2025

View reviewed changes

kkrik-es approved these changes Feb 12, 2025

View reviewed changes

[CI] Auto commit changes from spotless

d1019f8

martijnvg commented Feb 12, 2025

View reviewed changes

martijnvg added 3 commits February 12, 2025 11:07

iter

09e3988

remove redundant test code

44ceea7

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

25ef316

martijnvg enabled auto-merge (squash) February 13, 2025 08:01

martijnvg added 2 commits February 13, 2025 13:55

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

297eccd

Merge remote-tracking branch 'es/main' into logsdb_search_only_snapshots

cad4a03

martijnvg merged commit 1b452c7 into elastic:main Feb 13, 2025
17 checks passed

martijnvg added the backport pending label Feb 14, 2025

martijnvg mentioned this pull request Feb 14, 2025

[8.x] Logsdb and source only snapshots. #122572

Merged

martijnvg mentioned this pull request Feb 14, 2025

[9.0] Logsdb and source only snapshots. #122574

Merged

martijnvg removed the backport pending label Feb 14, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logsdb and source only snapshots. #122199

Logsdb and source only snapshots. #122199

martijnvg commented Feb 10, 2025 •

edited

Loading

elasticsearchmachine commented Feb 10, 2025

elasticsearchmachine commented Feb 10, 2025

kkrik-es Feb 11, 2025

kkrik-es Feb 11, 2025

martijnvg Feb 11, 2025

kkrik-es Feb 12, 2025

kkrik-es Feb 11, 2025

kkrik-es left a comment

kkrik-es Feb 12, 2025

martijnvg Feb 12, 2025

Logsdb and source only snapshots. #122199

Logsdb and source only snapshots. #122199

Conversation

martijnvg commented Feb 10, 2025 • edited Loading

elasticsearchmachine commented Feb 10, 2025

elasticsearchmachine commented Feb 10, 2025

kkrik-es Feb 11, 2025

Choose a reason for hiding this comment

kkrik-es Feb 11, 2025

Choose a reason for hiding this comment

martijnvg Feb 11, 2025

Choose a reason for hiding this comment

kkrik-es Feb 12, 2025

Choose a reason for hiding this comment

kkrik-es Feb 11, 2025

Choose a reason for hiding this comment

kkrik-es left a comment

Choose a reason for hiding this comment

kkrik-es Feb 12, 2025

Choose a reason for hiding this comment

martijnvg Feb 12, 2025

Choose a reason for hiding this comment

martijnvg commented Feb 10, 2025 •

edited

Loading