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

Increase cache refresh interval to 1 hour #912

Merged
merged 3 commits into from
Sep 22, 2020

Conversation

andrzej-stencel
Copy link
Contributor

The cache TTL was increased as well because it always needs
to be greater than the refresh interval, to prevent a situation where
there is no cached value for a resource.

The cache TTL was increased as well because it always needs
to be greater than the refresh interval, to prevent a situation where
there is no cached value for a resource.
@pmalek-sumo
Copy link
Contributor

2 small philosophical questions:

  • The cache TTL was increased as well...

    as well to? as well as? Maybe we're missing a reference PR there?

  • wouldn't increasing TTL make values in the cache invalid? (not sure how it works - just asking)

@perk-sumo perk-sumo added this to the v1.3 milestone Sep 16, 2020
@andrzej-stencel
Copy link
Contributor Author

Thanks @pmalek-sumo for this remark, the PR description was probably not detailed enough. Hopefully the below will be more descriptive, please let me know.

This PR increases default values for two parameters of the enhance_k8s_metadata plugin for Fluentd:

  • cache_refresh - increased from 30 minutes to 1 hour,
  • cache_ttl - increased from 1 hour to 2 hours.

The cache_refresh value has been increased because we want to perform less calls to k8s API server (or make the calls less often).

The cache_ttl value has been increased because we want this value to be always bigger than the cache_refresh value, to make sure the cache is refreshed sooner than it goes stale. This is to make sure the cache never goes stale and is always up-to-date.

Copy link
Contributor

@pmalek-sumo pmalek-sumo left a comment

Choose a reason for hiding this comment

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

So this change will decrease the number of API calls by a half. The question is: will this be enough for the user or should we investigate further?

@andrzej-stencel
Copy link
Contributor Author

We should definitely investigate further. This PR is just a start.

@andrzej-stencel andrzej-stencel merged commit 26982f2 into master Sep 22, 2020
@andrzej-stencel andrzej-stencel deleted the astencel-increase-cache-refresh-interval branch September 22, 2020 12:56
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.

None yet

3 participants