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

[CHANGE] regexp . to match \n and optimize performance #702

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ywwg
Copy link
Member

@ywwg ywwg commented Sep 20, 2024

This is backported from Prometheus 3.0 prometheus/prometheus#14505

Shortcut for .* matches newlines as well.
Add preamble change ^(?s:
Add test
dotAll flag por al regex
Add and fix regex tests

Co-authored-by: Mario Fernandez [email protected]

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

marioferh and others added 2 commits September 20, 2024 14:01
Shortcut for `.*` matches newlines as well.
Add preamble change ^(?s:
Add test
dotAll flag por al regex
Add and fix regex tests

Signed-off-by: Mario Fernandez <[email protected]>
@ywwg ywwg marked this pull request as ready for review September 20, 2024 18:06
@ywwg ywwg changed the title Fix: optimize .* regexp performance [CHANGE] regexp . to match \n and optimize performance Sep 20, 2024
@ywwg
Copy link
Member Author

ywwg commented Sep 20, 2024

I have signed the agreement although the assistant seems to think I have not

@ywwg
Copy link
Member Author

ywwg commented Sep 20, 2024

Also I can either squash the commits down or leave them separate... not sure how important it is to keep the commits in mimir-prometheus relatively close to upstream.

@56quarters
Copy link
Contributor

Also I can either squash the commits down or leave them separate... not sure how important it is to keep the commits in mimir-prometheus relatively close to upstream.

If they are separate in Prometheus main or the release branch, leave them separate here. It makes it easier to apply further changes.

@pracucci
Copy link
Collaborator

Hold on before merging. Not sure we're ready to merge this in main. There was a discussion in the last Mimir weekly about the breaking changes being / not being merged in mimir-prometheus main. Better if you kick off a discussion in #mimir-dev first.

@ywwg ywwg marked this pull request as draft September 20, 2024 19:08
@ywwg
Copy link
Member Author

ywwg commented Sep 20, 2024

converted to draft so nobody gets tempted to push the button

@ywwg
Copy link
Member Author

ywwg commented Sep 20, 2024

(I'll start the discussion on monday, no need to do so on a friday afternoon)

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