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

Add new track for tsdb based on k8s integration #373

Merged
merged 97 commits into from
Jun 9, 2023

Conversation

martijnvg
Copy link
Member

@martijnvg martijnvg commented Feb 2, 2023

This track indexes generated k8s pod and k8s container data based on the k8s integration and then test performance of searches that are based on k8s integration visualisations. See README for more details.

@martijnvg martijnvg marked this pull request as ready for review May 25, 2023 13:41
@pquentin pquentin requested a review from a team May 25, 2023 13:46
Copy link
Member

@inqueue inqueue left a comment

Choose a reason for hiding this comment

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

This is looking good, thank you. I left some README suggestions and have not executed the track yet.

Can you add track parameters for number_of_shards and number_of_replicas, if it makes sense to do so?

tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
martijnvg and others added 5 commits June 5, 2023 07:39
Co-authored-by: Jason Bryan <[email protected]>
Co-authored-by: Jason Bryan <[email protected]>
Co-authored-by: Jason Bryan <[email protected]>
Co-authored-by: Jason Bryan <[email protected]>
@martijnvg
Copy link
Member Author

Thanks for taking a look @inqueue. I updated the track.

Can you add track parameters for number_of_shards and number_of_replicas, if it makes sense to do so?

I think it makes sense and I added the track params.

Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

This is a great and awesome amount of work, thank you!

I have been able to run this benchmark (it's ongoing, will report when it succeeds).

Left a suggestion to clarify a little about what the touch-... tasks are doing and also clarify some sentences in the readme.

tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM

Left two comments, one for a minor typo, and another for a suggestion to move all the variables in track.json for better discoverability. Up to you if you want to implement the suggestion.

tsdb_k8s_queries/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,1085 @@
{% set end_time = "2023-05-16T21:00:00.000Z" %}
{% set time_intervals = {"15_minutes": ["30s", "2023-05-16T20:45:00.000Z"], "2_hours": ["1m", "2023-05-16T19:00:00.000Z"], "24_hours": ["30m", "2023-05-15T21:00:00.000Z"]} %}
Copy link
Contributor

Choose a reason for hiding this comment

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

The above two seem to be used also in challenges/default.json; does it make more sense to move them, as well as the variables from challenges/default.json:

{% set search_iterations = 20 %} 
{% set search_warmup_iterations = 50 %}
{% set target_interval = 4 %}

to a centralized place, e.g. at the top of track.json (right after {% import "rally.helpers" as rally with context %}) for better discoverability? If you choose to do that, remember to update the references in README.md.

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sense to me. I will update this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes sense to me. I will update this PR.

LGTM also after 5ea2206

Co-authored-by: Dimitrios Liappis <[email protected]>
@dliappis dliappis requested a review from inqueue June 6, 2023 12:00
Copy link
Member

@inqueue inqueue left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@martijnvg martijnvg merged commit 39882e9 into elastic:master Jun 9, 2023
inqueue pushed a commit to inqueue/rally-tracks that referenced this pull request Dec 6, 2023
This track indexes generated k8s pod and k8s container data based on the k8s integration and then test performance of searches that are based on k8s integration visualisations. See README for more details.
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.

5 participants