Synthetic id upgrade test in serverless by burqen · Pull Request #142471 · elastic/elasticsearch

burqen · 2026-02-13T12:55:09Z

Move TSDBSyntheticIdUpgradeIT to x-pack/plugin/logsdb/qa/rolling-upgrade. This module is shadowed in serverless which means the test will also run in serverless environment.

New parent class does not take care of upgrading the nodes, so this is added to TSDBSyntheticIdUpgradeIT, otherwise the test logic is unchanged.

Relates ES-14000
Continuation of #141525

Move TSDBSyntheticIdUpgradeIT to x-pack/plugin/logsdb/qa/rolling-upgrade This module is shadowed in serverless which means the test will also run in serverless environment. New parent class does not take care of upgrading the nodes, so this is added to TSDBSyntheticIdUpgradeIT, otherwise the test logic is unchanged.

elasticsearchmachine · 2026-02-13T12:55:34Z

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

Copilot

Pull request overview

Moves the TSDBSyntheticIdUpgradeIT rolling-upgrade coverage into the LogsDB rolling-upgrade QA module so it also runs in the serverless-shadowed variant, and adapts the test to perform node upgrades itself (since the new base class doesn’t parameterize upgrade phases).

Changes:

Relocated/adapted TSDBSyntheticIdUpgradeIT to extend AbstractLogsdbRollingUpgradeTestCase and run upgrades in-test via upgradeNode(i).
Added getClusterIndexVersion() helper to AbstractLogsdbRollingUpgradeTestCase to determine the (uniform) cluster index version before upgrades.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/TSDBSyntheticIdUpgradeIT.java	Switches to LogsDB rolling-upgrade base class and rewrites test flow to perform rolling upgrades directly.
x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/AbstractLogsdbRollingUpgradeTestCase.java	Adds a helper to read/assert a uniform cluster `IndexVersion` from `_nodes` for pre-upgrade branching logic.

Comments suppressed due to low confidence (2)

x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/TSDBSyntheticIdUpgradeIT.java:51

Comment grammar: "Cluster support" should be "Cluster supports" (or rephrase) to read correctly.
x-pack/plugin/logsdb/qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/TSDBSyntheticIdUpgradeIT.java:35
Avoid referencing the inherited static field cluster directly; prefer getCluster() (or qualify with AbstractLogsdbRollingUpgradeTestCase.cluster) so the access is explicit and future overrides/test harness changes are easier.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

martijnvg · 2026-02-13T16:01:28Z

...g-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/TSDBSyntheticIdUpgradeIT.java

-
    public void testRollingUpgrade() throws IOException {
-        IndexVersion oldClusterIndexVersion = getOldClusterIndexVersion();
+        IndexVersion oldClusterIndexVersion = getClusterIndexVersion();


Is there a test node feature for synthetic id? Some other bwc intregation tests do:

assumeTrue("...", oldClusterHasFeature(MY_CONSTANT));

If there isn't a node feature, then I think you can use something like: gte_v9.4.0? But maybe it is better to use a node feature, this makes not running integration tests in other contexts easier.

I've added a test node feature in this commit ba04200

Adding the node feature and relying on for this branch makes the test fail when run against main (9.4.0-snapshot) because main doesn't have the node feature but has support for synthetic id so test expect an exception but cluster happily create index.

I will add the node feature in a different branch and then update this test once the node feature has been merged to avoid this in between state.

👍 , so when that is merged, then this test can just do:

assumeTrue("old cluster needs to support synthetic id", oldClusterHasFeature(IndexFeatures.TIME_SERIES_SYNTHETIC_ID));

and then there should be no need to check index version and the getClusterIndexVersion() helper method.

The most important part of this test is to verify the behavior when upgrading from a version without to a version with the feature and make sure we fail in a way that is expected (fail to create index with reasonable exception message). So assumeTrue would be counter productive 😌

The only thing the version is used for otherwise is the exception message verification but maybe that's not a good enough reason to introduce the getClusterIndexVersion? I'll see what I can do to remove it 👍

Add IndexFeatures.TIME_SERIES_SYNTHETIC_ID and use it in TSDBSyntheticIdUpgradeIT instead of checking index version, so BWC tests can use oldClusterHasFeature() like other rolling upgrade tests. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

This reverts commit ba04200.

…n-serverless

…exVersion - Branch test on oldClusterHasFeature(IndexFeatures.TIME_SERIES_SYNTHETIC_ID) - Relax assertNoWriteIndex: verify illegal_argument_exception and setting rejection message without depending on index version text - Remove getClusterIndexVersion and getOldClusterVersion from base class Co-authored-by: Cursor <cursoragent@cursor.com>

martijnvg

LGTM, thanks @burqen!

…tatus randomLongBetween(0, 1000) can yield 0, which triggers the assertion in IndexShardSnapshotStatus.moveToDone that startTimeMillis != 0. Use randomLongBetween(1, 1000) so the test satisfies the invariant. Co-authored-by: Cursor <cursoragent@cursor.com>

fcofdez

LGTM. I left a minor comment.

fcofdez · 2026-02-18T09:09:45Z

server/src/test/java/org/elasticsearch/snapshots/SnapshotShutdownProgressTrackerTests.java

        switch (stage) {
            case DONE -> {
-                final long startTimeMillis = randomLongBetween(0, 1000);
+                final long startTimeMillis = randomLongBetween(1, 1000);


is this change intended?

Yes, this fixes a test failure. From commit message (beautifully composed by Cursor):

randomLongBetween(0, 1000) can yield 0, which triggers the assertion in IndexShardSnapshotStatus.moveToDone that startTimeMillis != 0. Use randomLongBetween(1, 1000) so the test satisfies the invariant.

Maybe we can move this into its own commit/PR?

fcofdez · 2026-02-18T09:13:29Z

...g-upgrade/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/TSDBSyntheticIdUpgradeIT.java

-                assertIndexRead("second-mixed-cluster-index");
+        int numNodes = getCluster().getNumNodes();
+
+        if (oldClusterHasFeature(IndexFeatures.TIME_SERIES_SYNTHETIC_ID)) {


The feature only guards the test execution?

Yes, it's a substitute for checking index version directly used by other tsdb tests and suggested by @martijnvg . The node feature was introduced in #142581 and only exist in 9.4 and later which makes it work the same way as checking index version directly. Maybe Martijn can elaborate on why this is preferred?

I think index versions can also be used, but cluster features are a more straight forward way to determine whether a cluster supports a specific feature? No need to introduce logic to extract index versions and the cluster feature testing infrastructure (e.g. oldClusterHasFeature(...)) can just be reused.

Thanks for clarifying Martijn!

…r DONE status" This reverts commit e751794.

…etic-id-rolling-upgrade-in-serverless

burqen · 2026-02-20T10:36:46Z

Waiting for #142581 to be promoted to production until merging this.

…etic-id-rolling-upgrade-in-serverless

fcofdez · 2026-02-24T12:43:52Z

can't we merge this @burqen?

burqen · 2026-02-24T16:13:50Z

can't we merge this @burqen?

~~It looks like the node feature has finally been promoted to the whole serverless pipeline so we are close!~~ The syntehtic id index feature (#142581) need to be promoted all the way to prod first. Otherwise the test will fail with "expected to throw but nothing was thrown" when testing against serverless prod.

I triggered a new build so check, but I don't think it has been promoted yet.

EDIT: Still waiting for promotion

…n-serverless

…etic-id-rolling-upgrade-in-serverless

…n-serverless

…etic-id-rolling-upgrade-in-serverless

And also adapt to use clusterRollingUpgrade instead of calling upgradeNode directly.

…n-serverless

coderabbitai · 2026-03-06T07:27:09Z

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (2)

Team:Delivery
Team:Search - Inference

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 0e269b49-07ba-4a98-b842-e44afd5a3fe6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

fcofdez

LGTM

Move TSDBSyntheticIdUpgradeIT to x-pack/plugin/logsdb/qa/rolling-upgrade This module is shadowed in serverless which means the test will also test rolling upgrade in serverless. * Use TIME_SERIES_SYNTHETIC_ID node feature in upgrade test, remove IndexVersion * Avoid analyse disk size in serverless, because that api is not available * Add logging to track failure * Add writing to all existing indices throughout the rolling upgrade --------- Co-authored-by: Cursor <cursoragent@cursor.com>

…locations * upstream/main: (153 commits) ES|QL: Update docs for TOP_SNIPPETS and DECAY (elastic#143739) Correctly include endpoint id in log msg in AuthorizationPoller (elastic#143743) Bar searching or sorting on _seq_no when disabled (elastic#143600) Generalize `testClientCancellation` test (elastic#143586) JSON_EXTRACT: zero-copy byte slicing for object, array, and number extraction (elastic#143702) Track recycler pages in circuit breaker (elastic#143738) [ESQL] Enable distributed pipeline breakers for external sources via FragmentExec (elastic#143696) Adding 'mode' and 'codec' fields to ES monitoring template (elastic#143673) [ESQL] Columnar I/O and vectorized block conversion for external sources (elastic#143703) Fix flaky MMR diversification YAML tests (elastic#143706) ES|QL codegen: check builder arguments for vector support (elastic#143724) Add Views Security Model (elastic#141050) ESQL: Prevent pushdown of unmapped fields in filters and sorts (elastic#143460) Don't run seq_no pruning tests in release CI (elastic#143725) ESQL: Support intra-row field references in ROW command (elastic#140217) ES|QL: Remove implicit limit in FORK branches in CSV tests (elastic#143601) IndexRoutingTests with and without synthetic id (elastic#143566) Synthetic id upgrade test in serverless (elastic#142471) Disable "Review skipped" comments for PRs without specified labels (elastic#143728) Cleanup ES|QL T-Digest code duplication, add memory accounting (elastic#143662) ...

Move TSDBSyntheticIdUpgradeIT to x-pack/plugin/logsdb/qa/rolling-upgrade This module is shadowed in serverless which means the test will also test rolling upgrade in serverless. * Use TIME_SERIES_SYNTHETIC_ID node feature in upgrade test, remove IndexVersion * Avoid analyse disk size in serverless, because that api is not available * Add logging to track failure * Add writing to all existing indices throughout the rolling upgrade --------- Co-authored-by: Cursor <cursoragent@cursor.com>

burqen added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics v9.4.0 labels Feb 13, 2026

burqen requested review from fcofdez, martijnvg and tlrx February 13, 2026 12:55

elasticsearchmachine added the Team:StorageEngine label Feb 13, 2026

burqen requested a review from Copilot February 13, 2026 12:55

Copilot started reviewing on behalf of burqen February 13, 2026 12:56 View session

burqen mentioned this pull request Feb 13, 2026

Rolling upgrade test for synthetic id #141525

Merged

Copilot AI reviewed Feb 13, 2026

View reviewed changes

martijnvg reviewed Feb 13, 2026

View reviewed changes

burqen and others added 5 commits February 16, 2026 10:35

Use getCluster() instead of inherited static cluster field

4604a64

Co-authored-by: Cursor <cursoragent@cursor.com>

Revert "Introduce test node feature for time series synthetic ID"

6494e77

This reverts commit ba04200.

Merge upstream main into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

9b531ab

…n-serverless

martijnvg approved these changes Feb 18, 2026

View reviewed changes

fcofdez approved these changes Feb 18, 2026

View reviewed changes

burqen added 3 commits February 18, 2026 13:28

Revert "Fix testTrackerClearShutdown: use non-zero startTimeMillis fo…

58446ab

…r DONE status" This reverts commit e751794.

Avoid analyse disk size in serverless

5bd36b5

Merge remote-tracking branch 'upstream/main' into ap/2026.02.13.synth…

e847bbc

…etic-id-rolling-upgrade-in-serverless

burqen mentioned this pull request Feb 23, 2026

[CI] TSDBSyntheticIdUpgradeIT testRollingUpgrade {upgradedNodes=3} failing #141749

Closed

burqen added 2 commits February 23, 2026 14:43

Improve exception assertion

40d2cf3

Merge remote-tracking branch 'upstream/main' into ap/2026.02.13.synth…

28b582d

…etic-id-rolling-upgrade-in-serverless

burqen and others added 11 commits February 24, 2026 17:14

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

17f8914

…n-serverless

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

b58d9d2

…n-serverless

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

08d511c

…n-serverless

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

1ba9a04

…n-serverless

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

bb5e7d3

…n-serverless

Merge remote-tracking branch 'upstream/main' into ap/2026.02.13.synth…

437b621

…etic-id-rolling-upgrade-in-serverless

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

ad91135

…n-serverless

Add logging to track failure

60f1ab4

Merge remote-tracking branch 'upstream/main' into ap/2026.02.13.synth…

cd17c85

…etic-id-rolling-upgrade-in-serverless

Add writing to all existing indices

233b8dc

And also adapt to use clusterRollingUpgrade instead of calling upgradeNode directly.

Merge branch 'main' into ap/2026.02.13.synthetic-id-rolling-upgrade-i…

d4694ad

…n-serverless

fcofdez approved these changes Mar 6, 2026

View reviewed changes

burqen merged commit cc5852b into elastic:main Mar 6, 2026
36 checks passed

prwhelan mentioned this pull request Mar 6, 2026

[ML] Wait for cluster state in test #143767

Merged

prwhelan mentioned this pull request Mar 9, 2026

[Transform] Disable PIT for CPS #143876

Closed

Conversation

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

Uh oh!

Uh oh!

elasticsearchmachine commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

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!

Choose a reason for hiding this comment

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

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!

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

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!

Choose a reason for hiding this comment

Uh oh!

burqen commented Feb 20, 2026

Uh oh!

fcofdez commented Feb 24, 2026

Uh oh!

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

Uh oh!

Uh oh!

coderabbitai bot commented Mar 6, 2026

Review skipped

Uh oh!

fcofdez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

burqen commented Feb 13, 2026 •

edited

Loading

fcofdez Feb 18, 2026 •

edited

Loading

burqen commented Feb 24, 2026 •

edited

Loading