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

#minor Add an option to send request only once per batch #356

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ahtohio
Copy link

@ahtohio ahtohio commented Aug 16, 2024

Reloading on each received event (with watch method) can be quite costly. I run Grafana provisioning and asking it to reload all my provisioned configmaps (>300) on each event is not needed and only increases cpu load.

Reload once after each WATCH_SERVER_TIMEOUT seconds should be enough.

I've added an option REQ_ONCE_PER_BATCH that can toggle this reload-once-per-watch-cycle reload behaviour, false by default to not interfere with existing logic.

@ChristianGeie
Copy link
Collaborator

Hi @ahtohio, thx for contributing. I like your reload idea to save cpu time. Changes looks good to me so far.

Do you have any ideas about whether the function can be included into the tests?

@ahtohio ahtohio marked this pull request as draft September 10, 2024 11:36
@ahtohio ahtohio marked this pull request as ready for review September 10, 2024 13:13
@ahtohio
Copy link
Author

ahtohio commented Sep 10, 2024

Hi, @ChristianGeie
I've added some tests related to this functionality. For some reason, at the last commit all jobs failed for quite unknown reasons, but new workflow on the same commit in my personal branch worked well.
But I can't restart/create new workflow in this repo, it seems

@ChristianGeie ChristianGeie added enhancement New feature or request python Pull requests that update Python code labels Sep 16, 2024
@ChristianGeie
Copy link
Collaborator

thx @ahtohio . I do a lot of changes regarding workflow actions in the last couple of weeks, please excuse that sometimes the tests didn't work reliably.
Otherwise your changes look pretty good to me.

@ahtohio
Copy link
Author

ahtohio commented Oct 8, 2024

Hi, @ChristianGeie, it seems all's good, only approval left

@isubasinghe
Copy link

@ChristianGeie this is a pretty important fix in my opinion, my 12 core CPU gets hammered while running our application.

If there is anything I can do to progress this PR, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants