Support nested documents in time-series indices with synthetic id by tlrx · Pull Request #143151 · elastic/elasticsearch

tlrx · 2026-02-26T15:01:46Z

This change adds the doc values fields that are required for synthetic id to work in nested documents. It also fixes an assertion that tripped during tests execution.

The changes in TSDBSyntheticIdsIT allows to run the existing tests with nested docs in 10% executions (it uses rarely(), should we increase this? I have no idea if nested docs are widely used in time-series).

Also, I'm chasing a flaky test in testRecoveredOperations but I think we can move forward with the reviews.

Relates ES-14224

elasticsearchmachine · 2026-02-26T15:02:43Z

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

elasticsearchmachine · 2026-02-27T08:37:11Z

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

fcofdez

Looks good, I left a couple of comments.

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java

fcofdez · 2026-02-27T10:03:00Z

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/160_nested_fields.yml

      cluster_features: ["mapper.tsdb_nested_field_support"]
      reason: "tsdb index with nested field support enabled"

+  - skip:


Would this test suite be executed anytime then?

I think it would be executed on snapshot builds today, not on release tests. Once we enable the feature by default then we can remove the test suite and only rely on the two other.

The yml tests run in mixed cluster environments and the cluster_feature is only "present" if all members of the cluster has the feature, so this test will run in all cluster combinations pre-9.4.0. So basically

Mixed cluster with old version <9.4.0, run this test.
Mixed cluster with old version >=9.4.0 run the other two.
I think of cluster_feature as a shortcut to check max shared IndexVersion across cluster.

martijnvg · 2026-02-27T10:32:09Z

I have no idea if nested docs are widely used in time-series

I have never seen nested usage in tsdb. But it is possible to configure, so I think this change make sense. Especially later when we plan to reuse synthetic id for other index modes like logsdb.

martijnvg

Looks good! I left a question.

martijnvg · 2026-02-27T13:37:27Z

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java

+            final var parentTimeSeriesId = parentTsIdField.binaryValue();
+            assert parentTimeSeriesId.equals(timeSeriesId);
+            assert parentTimeSeriesId.equals(TsidExtractingIdFieldMapper.extractTimeSeriesIdFromSyntheticId(uidEncoded));
+            if (this.useDocValuesSkipper) {


The default is that useDocValuesSkipper is enabled for tsdb. Users would need to manually set index.mapping.use_doc_values_skipper to false. Did tests fail if these checks were not added? Just out of curiosity.

Users would need to manually set index.mapping.use_doc_values_skipper to false

I thought that was some undocumented setting that users would not modify. But if they can, then we should also run our tests with doc values skippers rarely disabled, do you agree?

Asking because I haven't randomized this setting so far, assuming it would be removed and always enabled in a short future.

Did tests fail if these checks were not added?

I would expect the tests to pass but I haven't tried for the previous reason.

Quickly trying on TSDBSyntheticIdsIT returns a lot of failures.

This setting is reused in logsdb, but there it is disabled by default and we do document this setting iirc.
I don't think we have to support the case were index.mapping.use_doc_values_skipper is disabled. Maybe in another change we can add validation that prevents the use of synthetic id if this this setting is disabled?

Maybe in another change we can add validation that prevents the use of synthetic id if this this setting is disabled?

Thanks for the details. I think we can support use_doc_values_skipper to be disabled with a couple of extra changes that I can do if a follow up. That would be easier to maintain compare to preventing the use of synthetic id when doc values skippers are disabled.

I opened #143389 to support disabling the doc values skippers with TSDB synthetic ids.

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java

This change disables `index.mapping.use_doc_values_skipper` in about 10% of tests executions of the main TSDB with synthetic id tests, as well as in some unit tests. Relates ES-14224 Relates elastic#143151 (comment)

fcofdez

LGTM

fcofdez · 2026-03-03T09:28:20Z

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java

+            assert nestedDoc.getField(TimeSeriesRoutingHashFieldMapper.NAME) == null;
+
+            if (rootParentDoc != null) {
+                assert nestedDocFields.isEmpty() == false;


nit: maybe we can assert that nestedDocFields size is 4?

I pushed 0777769

burqen

Changes look good. I just had some clarifying comments around cluster feature and suggestions for javadoc improvements. Great job!

.../src/yamlRestTest/resources/rest-api-spec/test/tsdb/161_nested_fields_synthetic_id_false.yml

...c/src/yamlRestTest/resources/rest-api-spec/test/tsdb/162_nested_fields_synthetic_id_true.yml

burqen · 2026-03-03T09:34:43Z

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/tsdb/160_nested_fields.yml

      cluster_features: ["mapper.tsdb_nested_field_support"]
      reason: "tsdb index with nested field support enabled"

+  - skip:


The yml tests run in mixed cluster environments and the cluster_feature is only "present" if all members of the cluster has the feature, so this test will run in all cluster combinations pre-9.4.0. So basically

Mixed cluster with old version <9.4.0, run this test.
Mixed cluster with old version >=9.4.0 run the other two.
I think of cluster_feature as a shortcut to check max shared IndexVersion across cluster.

server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

burqen · 2026-03-03T12:25:20Z

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java

        return CONTENT_TYPE;
    }

+    private void addSyntheticIdFieldsToNestedDocs(DocumentParserContext context, BytesRef timeSeriesId, BytesRef uidEncoded) {


Great assertions in this method! 👍

martijnvg

LGTM

tlrx · 2026-03-05T07:46:41Z

Thanks everyone!

…astic#143151) This change adds the doc values fields that are required for synthetic id to work in nested documents. It also fixes an assertion that tripped during tests execution. The changes in TSDBSyntheticIdsIT allows to run the existing tests with nested docs in 10% executions (it uses rarely()). Relates ES-14224

tlrx added 2 commits February 26, 2026 13:56

add synthetic id fields to nested docs

22b6ad5

add yaml tests

33855bf

elasticsearchmachine added the v9.4.0 label Feb 26, 2026

tlrx added >enhancement :StorageEngine/TSDB You know, for Metrics labels Feb 26, 2026

Update docs/changelog/143151.yaml

ca8e278

elasticsearchmachine and others added 4 commits February 26, 2026 15:10

[CI] Auto commit changes from spotless

ab23f43

fix

710b6e8

Merge branch 'main' into 2026/02/26-es-14224

5932729

Merge branch 'main' into 2026/02/26-es-14224

08caa8c

tlrx marked this pull request as ready for review February 27, 2026 08:36

tlrx requested review from burqen, fcofdez and martijnvg February 27, 2026 08:36

elasticsearchmachine added the Team:StorageEngine label Feb 27, 2026

fcofdez reviewed Feb 27, 2026

View reviewed changes

martijnvg reviewed Feb 27, 2026

View reviewed changes

Merge branch 'main' into 2026/02/26-es-14224

e2b72ae

martijnvg reviewed Mar 2, 2026

View reviewed changes

server/src/main/java/org/elasticsearch/index/mapper/TimeSeriesIdFieldMapper.java Show resolved Hide resolved

nested doc fields

e8f6c49

tlrx force-pushed the 2026/02/26-es-14224 branch from 06439d7 to e8f6c49 Compare March 2, 2026 09:46

fix assertion

ac274c4

tlrx mentioned this pull request Mar 2, 2026

[Test] Randomly disable doc values skippers in time-series indices #143389

Merged

Merge branch 'main' into 2026/02/26-es-14224

aa57f91

tlrx requested a review from fcofdez March 3, 2026 09:16

tlrx requested a review from martijnvg March 3, 2026 09:16

fcofdez approved these changes Mar 3, 2026

View reviewed changes

tlrx added 2 commits March 3, 2026 11:24

feedback

0777769

Merge branch 'main' into 2026/02/26-es-14224

cceffbd

burqen approved these changes Mar 3, 2026

View reviewed changes

tlrx requested a review from romseygeek March 3, 2026 15:37

tlrx added 3 commits March 3, 2026 16:57

feedback

86c8e40

docs

e29e37a

Merge branch 'main' into 2026/02/26-es-14224

ef9c0c7

romseygeek approved these changes Mar 4, 2026

View reviewed changes

martijnvg approved these changes Mar 4, 2026

View reviewed changes

Merge branch 'main' into 2026/02/26-es-14224

392caaf

tlrx merged commit ec5e325 into elastic:main Mar 5, 2026
35 checks passed

tlrx deleted the 2026/02/26-es-14224 branch March 5, 2026 07:46

burqen mentioned this pull request Mar 6, 2026

Enable synthetic id feature, remove feature flag #142366

Closed

16 tasks

This was referenced Mar 6, 2026

[ML] Wait for cluster state in test #143767

Merged

[Transform] Disable PIT for CPS #143876

Closed

Conversation

tlrx commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Feb 26, 2026

Uh oh!

elasticsearchmachine commented Feb 27, 2026

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnvg commented Feb 27, 2026

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

burqen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tlrx commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tlrx commented Feb 26, 2026 •

edited

Loading