Skip to content

[Test] Randomly disable sequence numbers in CcrTimeSeriesDataStreamsIT#143930

Merged
tlrx merged 9 commits intoelastic:mainfrom
tlrx:2026/03/10-disable-seq-no-in-ccrtimeseriesdatastreamsit
Mar 13, 2026
Merged

[Test] Randomly disable sequence numbers in CcrTimeSeriesDataStreamsIT#143930
tlrx merged 9 commits intoelastic:mainfrom
tlrx:2026/03/10-disable-seq-no-in-ccrtimeseriesdatastreamsit

Conversation

@tlrx
Copy link
Copy Markdown
Member

@tlrx tlrx commented Mar 10, 2026

This change randomly disables sequence numbers in CcrTimeSeriesDataStreamsIT so that it runs with synthetic id too.

Note: We don't do extensive checks to avoid CcrTimeSeriesDataStreamsIT do too many checks at a time. I'll follow up with a dedicated CCR integration test for disabled sequence numbers on regular indices where we'll check that seq no are pruned away from segments.

Relates #136305

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. v9.4.0 labels Mar 10, 2026
@tlrx tlrx requested review from burqen, fcofdez and romseygeek March 10, 2026 08:55
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team. label Mar 10, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

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 👍

)
);
}
// Sequence numbers cannot be trimmed for points, so we enforce doc values only usage
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.

Do we want to remove this comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, I think it is still valid, we can't trim seq no if SEQ_NO_INDEX_OPTIONS_SETTING uses points. But the early exit here means that if a template set SEQ_NO_INDEX_OPTIONS_SETTING it is ignored until the index creation time :(

We could change the SEQ_NO_INDEX_OPTIONS_SETTING to also uses IndexVersion.ZERO but I'm a bit worried about potential impacts :(

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.

Yep, Kostas hit the same issue I think #143897 (comment)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing this, I'll wait for Kostas pull request to be merged then.

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.

I think we want to reinstate the comment here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I misread Francisco's initial comment, yes the comment should be reinstated and should not have been removed in my PR.

I pushed bc1b102

if (IndexSettings.TSDB_SYNTHETIC_ID_FEATURE_FLAG) {
settingsBuilder.put(IndexSettings.SYNTHETIC_ID.getKey(), useSyntheticId);
}
if (IndexSettings.DISABLE_SEQUENCE_NUMBERS_FEATURE_FLAG && randomBoolean()) {
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.

Does this need to be combined with the doc_values_only flag as well, or is that always set here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Time-series indices default to doc values only in recent versions so it is OK

@tlrx tlrx requested a review from romseygeek March 11, 2026 15:05
Copy link
Copy Markdown
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

One nit, LGTM otherwise.

)
);
}
// Sequence numbers cannot be trimmed for points, so we enforce doc values only usage
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.

I think we want to reinstate the comment here?

@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Mar 13, 2026

#144180 has been merged so I can resume merging this change.

@tlrx tlrx added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Mar 13, 2026
@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Mar 13, 2026

Tests are green, merging

@tlrx tlrx merged commit d15f808 into elastic:main Mar 13, 2026
35 of 36 checks passed
@tlrx tlrx deleted the 2026/03/10-disable-seq-no-in-ccrtimeseriesdatastreamsit branch March 13, 2026 18:18
szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 13, 2026
…elocations

* upstream/main: (72 commits)
  [Test] Randomly disable sequence numbers in CcrTimeSeriesDataStreamsIT (elastic#143930)
  Fix AsyncSearchIndexServiceTests.testCircuitBreaker failure (elastic#144058)
  Refine GenerativeIT some more, this time with accounting for some added (elastic#144220)
  ESQL: Physical Planning on the Lookup Node (elastic#143707)
  Mute org.elasticsearch.xpack.esql.CsvIT test {csv-spec:approximation.Approximate stats by with zero variance} elastic#144240
  Trigger counter metrics in test for delta temporality measurements (elastic#144193)
  fix capabiltiy approximation_v3 (elastic#144230)
  [ci] Add PR pipeline for testing ipv6 and fix tests not working with ipv6 (elastic#140473)
  update (elastic#144095)
  Make from/to optional in TBUCKET when Kibana timestamp filter is present (elastic#144057)
  Extract reroute behavior from create-index request classes (elastic#144140)
  ESQL: Fix release build only failures (elastic#144122)
  ES|QL query approximation: move sample correction to data node (elastic#144005)
  Add indexing pressure tracking to OTLP endpoints (elastic#144009)
  Fix replica writes after _seq_no doc values are pruned (elastic#144180)
  allow tests to configure supportsLoadingConfig (elastic#144061)
  [ES|QL] Unmute testGiantTextFieldInSubqueryIntermediateResultsWithSort (elastic#144126)
  [ESQL][DOCS] Add CPS page (unpublished for moment) (elastic#144206)
  ESQL: Forbid "load" unmapped_fields for certain commands (elastic#144115)
  Add CCS Remote Views Detection (elastic#143384)
  ...
ncordon pushed a commit to ncordon/elasticsearch that referenced this pull request Mar 16, 2026
elastic#143930)

This change randomly disables sequence numbers in CcrTimeSeriesDataStreamsIT so that it runs with synthetic id too.

Note: We don't do extensive checks to avoid CcrTimeSeriesDataStreamsIT do too many checks at a time. I'll follow up with a dedicated CCR integration test for disabled sequence numbers on regular indices where we'll check that seq no are pruned away from segments.

Relates elastic#136305
michalborek pushed a commit to michalborek/elasticsearch that referenced this pull request Mar 23, 2026
elastic#143930)

This change randomly disables sequence numbers in CcrTimeSeriesDataStreamsIT so that it runs with synthetic id too.

Note: We don't do extensive checks to avoid CcrTimeSeriesDataStreamsIT do too many checks at a time. I'll follow up with a dedicated CCR integration test for disabled sequence numbers on regular indices where we'll check that seq no are pruned away from segments.

Relates elastic#136305
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed/Engine Anything around managing Lucene and the Translog in an open shard. Team:Distributed Meta label for distributed team. >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.

4 participants