Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a new operation to benchmark the shard changes recovery api #722

Conversation

salvatore-campagna
Copy link
Contributor

@salvatore-campagna salvatore-campagna commented Jan 10, 2025

With PR elastic/elasticsearch#118608, we introduced a new API that we would like to benchmark in order to evaluate the latency of synthetic source recovery. This benchmarking will help us assess how the API latency evolves over time and understand the potential impact of new features on recovery latency. The targets for this benchmark will include indices or data streams in the elastic/logs, elastic/security, and tsdb tracks.

  • Retention Lease Requirement:
    It is important to add a retention lease before starting document indexing. This is necessary for ensuring that the subsequent shard recovery API call executes successfully without missing seq_no values. A missing retention lease may result in missing seq_no values during shard recovery.

  • Support for Data Streams and Index Aliases:
    We extended the original API in PR elastic/elasticsearch#118937 to support data streams and index aliases. This extension is necessary for data stream use cases, as the backing index name may change depending on when the index is created. Supporting data streams and aliases simplifies benchmarking and provides greater flexibility in targeting indices.

@salvatore-campagna salvatore-campagna marked this pull request as ready for review January 14, 2025 16:53
@salvatore-campagna salvatore-campagna self-assigned this Jan 14, 2025
Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Thanks Salvatore! I left a comment about inlining shard-recovery in existing challenge.

@salvatore-campagna salvatore-campagna marked this pull request as draft January 15, 2025 08:35
@gareth-ellis
Copy link
Member

@elasticmachine update branch

elastic/logs/track.json Show resolved Hide resolved
@salvatore-campagna salvatore-campagna changed the title Introduce a new challenge to benchmark the shard changes recovery api Introduce a new operation to benchmark the shard changes recovery api Jan 15, 2025
@salvatore-campagna salvatore-campagna marked this pull request as ready for review January 21, 2025 16:26
"index": "logs-*",
"operation": "refresh"
},
{
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to these operations related to merging? It just makes this challenge run longer?

"name": "refresh-after-index",
"operation": "refresh"
},
{
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove force-merge and wait-until-merges-finish here?

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM2

@salvatore-campagna salvatore-campagna merged commit 7ea9577 into elastic:master Jan 22, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants