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

Make event cache tunable #2928

Merged
merged 3 commits into from
Sep 18, 2024
Merged

Make event cache tunable #2928

merged 3 commits into from
Sep 18, 2024

Conversation

tpapagian
Copy link
Member

@tpapagian tpapagian commented Sep 16, 2024

This PR makes the event cache tunable. This includes the number of retries and the delay between each retry. For this reason we introduce 2 new command line flags:

      --event-cache-retries int                   Number of retries for event cache (default 15)
      --event-cache-retry-delay int               Delay in seconds between event cache retries (default 2)

This PR does not change the default values of those.

We also export those values through the Helm Chart.

Make eventCache number of retries and delay between them tunable.

Copy link

netlify bot commented Sep 16, 2024

Deploy Preview for tetragon ready!

Name Link
🔨 Latest commit 8a06e70
🔍 Latest deploy log https://app.netlify.com/sites/tetragon/deploys/66e7feb126d22c0008e361aa
😎 Deploy Preview https://deploy-preview-2928--tetragon.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@tpapagian tpapagian added the release-note/misc This PR makes changes that have no direct user impact. label Sep 16, 2024
@tpapagian tpapagian changed the title Make event cache tunable Make EventCache tunable Sep 16, 2024
@tpapagian tpapagian changed the title Make EventCache tunable Make eventCache tunable Sep 16, 2024
@tpapagian tpapagian changed the title Make eventCache tunable Make event cache tunable Sep 16, 2024
@tpapagian tpapagian force-pushed the pr/apapag/eventcache-conf branch 3 times, most recently from eb2d780 to 649da64 Compare September 16, 2024 10:41
@tpapagian tpapagian marked this pull request as ready for review September 16, 2024 11:13
@tpapagian tpapagian requested review from a team and mtardy as code owners September 16, 2024 11:13
Copy link
Member

@mtardy mtardy left a comment

Choose a reason for hiding this comment

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

Thanks! Lgtm! Out of curiosity, what was the reason behind this change?

pkg/jsonchecker/jsonchecker.go Outdated Show resolved Hide resolved
pkg/option/config.go Outdated Show resolved Hide resolved
These include the number of retries and the delay between them.

New command line arguments to do that:
      --event-cache-retries int                   Number of retries for event cache (default 15)
      --event-cache-retry-delay int               Delay in seconds between event cache retries (default 2)

Signed-off-by: Anastasios Papagiannis <[email protected]>
The previous patch makes EventCache configurable. This allows to reduce
some delays in the tests.

Signed-off-by: Anastasios Papagiannis <[email protected]>
Signed-off-by: Anastasios Papagiannis <[email protected]>
@tpapagian
Copy link
Member Author

Out of curiosity, what was the reason behind this change?

There may be cases where the k8s watcher is overloaded and the default settings may not enough. On the other hand, having a large delay may result on waiting too much for getting events (even without pod info). So I believe that this is a good idea to allow users to configure that based on their needs.

@tpapagian tpapagian merged commit 451f921 into main Sep 18, 2024
44 checks passed
@tpapagian tpapagian deleted the pr/apapag/eventcache-conf branch September 18, 2024 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants