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

Adjust elastic/logs update-custom-package-templates for serverless #573

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gbanasiak
Copy link
Contributor

Follow-up to #563 which missed to update update-custom-package-templates task based on the presence and value of include_non_serverless_index_settings track parameter.

We have 2 overlapping mechanisms for specifying index settings in elastic/logs:

The initial idea in #336 was to only trigger modification of pre-existing assets if specific track parameter is set, but ultimately an overlap scenario was chosen (see this comment). The plan that was never implemented was to get rid of static assets in favor of the custom runner (see this comment).

What I don't like about the custom runner approach is that it modifies all component templates ending with @custom in the cluster (src). What about benchmarks against live clusters? This should be an opt-in, not a default, right?

What I'm proposing in this PR is a logical extension to #563 which maintains the status quo, but I'm open to extending the scope a bit, and making update-custom-package-templates optional and disabled by default. WDYT?

@gbanasiak gbanasiak requested a review from a team February 23, 2024 13:31
@gbanasiak gbanasiak marked this pull request as ready for review March 1, 2024 15:36
Copy link
Member

@gareth-ellis gareth-ellis left a comment

Choose a reason for hiding this comment

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

The changes LGTM,

With regard to changing the behaviour and whether updating everything should be opt in or opt out, I'm a little unsure. We do say in various documentation that users of rally and rally-tracks have to be aware that some or all tracks may not be suitable for running on a production server, and instead should be benchmarking on a seperate cluster that they don't mind losing all the data. That said, this does behave slightly differently than others, whereby other tracks tend to just touch the indexes associated with the track.

I think I'd be happy either way, along with a comment in the README.md that points out what's happening

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.

2 participants