Skip to content

RandomizedTimeSeriesIT use synthetic id randomly#142956

Merged
burqen merged 1 commit intoelastic:mainfrom
burqen:ap/2026.02.24.synthetic-id-fix-RandomizedTimeSeriesIT
Feb 25, 2026
Merged

RandomizedTimeSeriesIT use synthetic id randomly#142956
burqen merged 1 commit intoelastic:mainfrom
burqen:ap/2026.02.24.synthetic-id-fix-RandomizedTimeSeriesIT

Conversation

@burqen
Copy link
Copy Markdown
Contributor

@burqen burqen commented Feb 24, 2026

Randomly use synthetic_id in RandomizedTimeSeriesIT.

Fix assertion in VersionsAndSeqNoResolver.
The uid is generated with Uid.encodeId(id), which can prepend BASE64_ESCAPE (e0xfd), see Uid.encodeBase64Id. When comparing to id encoded using Base64.getUrlDecoder().decode(id) directly, this would fail the assertion. Update assertion to use the same method of encoding as when the uid was first encoded.

Randomly use synthetic_id in RandomizedTimeSeriesIT.

Fix assertion in VersionsAndSeqNoResolver.
The uid is generated with Uid.encodeId(id), which can prepend
BASE64_ESCAPE (e0xfd), see Uid.encodeBase64Id. When comparing to id
encoded using Base64.getUrlDecoder().decode(id) directly, this would
fail the assertion. Update assertion to use the same method of encoding
as when the uid was first encoded.
@burqen burqen requested review from fcofdez and tlrx February 24, 2026 14:53
@burqen burqen added >test Issues or PRs that are addressing/adding tests :StorageEngine/TSDB You know, for Metrics v9.4.0 labels Feb 24, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

Comment on lines -174 to +175
assert uid.equals(new BytesRef(Base64.getUrlDecoder().decode(id)));
assert uid.equals(Uid.encodeId((id)));
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.

Please shout if there was a specific reason why we validated against new BytesRef(Base64.getUrlDecoder().decode(id)) specifically here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I just reused the decoder that was in place (byte[] idAsBytes = Base64.getUrlDecoder().decode(id); from the following lines) when I added the if (useSyntheticId) condition, but that was a mistake.

In this method we're looking up a Lucene term which is encoded using Uid.encodeId() so I think your change indeed fixes the assertion. I'm surprized in never failed so far.

Nice catch!

Copy link
Copy Markdown
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines -174 to +175
assert uid.equals(new BytesRef(Base64.getUrlDecoder().decode(id)));
assert uid.equals(Uid.encodeId((id)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I just reused the decoder that was in place (byte[] idAsBytes = Base64.getUrlDecoder().decode(id); from the following lines) when I added the if (useSyntheticId) condition, but that was a mistake.

In this method we're looking up a Lucene term which is encoded using Uid.encodeId() so I think your change indeed fixes the assertion. I'm surprized in never failed so far.

Nice catch!

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 644d851 into elastic:main Feb 25, 2026
35 checks passed
smalyshev pushed a commit to smalyshev/elasticsearch that referenced this pull request Feb 25, 2026
Randomly use synthetic_id in RandomizedTimeSeriesIT.

Fix assertion in VersionsAndSeqNoResolver.
The uid is generated with Uid.encodeId(id), which can prepend
BASE64_ESCAPE (e0xfd), see Uid.encodeBase64Id. When comparing to id
encoded using Base64.getUrlDecoder().decode(id) directly, this would
fail the assertion. Update assertion to use the same method of encoding
as when the uid was first encoded.
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.

4 participants