Skip to content

Enhance TSDB tests with synthetic ID support#143031

Merged
burqen merged 7 commits intoelastic:mainfrom
burqen:ap/2026.02.25.synthetic-id-fix-TsdbIndexingRollingUpgradeIT
Mar 5, 2026
Merged

Enhance TSDB tests with synthetic ID support#143031
burqen merged 7 commits intoelastic:mainfrom
burqen:ap/2026.02.25.synthetic-id-fix-TsdbIndexingRollingUpgradeIT

Conversation

@burqen
Copy link
Copy Markdown
Contributor

@burqen burqen commented Feb 25, 2026

TsdbIT and TsdbIndexingRollingUpgradeIT now runs with and without synthetic id randomly if the cluster has support it.

This change improves the test coverage for synthetic id and prepare the tests for synthetic id being enabled by default.

Also removed some magic strings

This is preparation for enabling synthetic id by default #142366

TsdbIT and TsdbIndexingRollingUpgradeIT now runs with and without synthetic id randomly
if the cluster has support it.

This change improves the test coverage for synthetic id and prepare the tests for
synthetic id being enabled by default.

Also removed some magic strings
@burqen burqen added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics v9.4.0 labels Feb 25, 2026
@burqen burqen requested review from fcofdez and tlrx February 25, 2026 09:28
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a minor comment

@@ -39,16 +43,25 @@ public class TsdbIndexingRollingUpgradeIT extends AbstractLogsdbRollingUpgradeTe
""";

public void testIndexing() throws Exception {
boolean hasSupport = oldClusterHasFeature(IndexFeatures.TIME_SERIES_SYNTHETIC_ID);
Boolean useSyntheticId = hasSupport ? randomBoolean() : null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we simply use and && here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to take care of the case when old nodes in the cluster lack support for synthetic id. That is the null case. I've added this clarifying comment to the test:

If cluster has support for synthetic id, randomly set index.mapping.synthetic_id to true/false
If cluster doesn't have support, don't set index.mapping.synthetic_id at all (null),
indicated by oldClusterHasFeature(IndexFeatures.TIME_SERIES_SYNTHETIC_ID)==false

Copy link
Copy Markdown
Contributor

@fcofdez fcofdez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@burqen burqen merged commit 881fb45 into elastic:main Mar 5, 2026
22 of 26 checks passed
burqen added a commit to burqen/elasticsearch that referenced this pull request Mar 5, 2026
TsdbIT and TsdbIndexingRollingUpgradeIT now runs with and without synthetic id randomly
if the cluster has support it.

This change improves the test coverage for synthetic id and prepare the tests for
synthetic id being enabled by default.

Also removed some magic strings

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Co-authored-by: Cursor
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Mar 5, 2026
TsdbIT and TsdbIndexingRollingUpgradeIT now runs with and without synthetic id randomly
if the cluster has support it.

This change improves the test coverage for synthetic id and prepare the tests for
synthetic id being enabled by default.

Also removed some magic strings

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Co-authored-by: Cursor
spinscale pushed a commit to spinscale/elasticsearch that referenced this pull request Mar 6, 2026
TsdbIT and TsdbIndexingRollingUpgradeIT now runs with and without synthetic id randomly
if the cluster has support it.

This change improves the test coverage for synthetic id and prepare the tests for
synthetic id being enabled by default.

Also removed some magic strings

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Co-authored-by: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:StorageEngine/TSDB You know, for Metrics Team:StorageEngine >test Issues or PRs that are addressing/adding tests v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants