Increase default value of cache TTL to 6 hours. #107141

mk46 · 2021-12-20T13:59:51Z

Increase default value of cache TTL to 6 hours.

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Increase default value of discovery cache TTL for kubectl to 6 hours.

k8s-ci-robot · 2021-12-20T13:59:59Z

Hi @mk46. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mk46 · 2021-12-20T14:00:32Z

/kind feature
/release-note-none

jonnylangefeld · 2022-01-02T04:40:11Z

staging/src/k8s.io/cli-runtime/pkg/genericclioptions/config_flags.go

@@ -439,13 +447,21 @@ func NewConfigFlags(usePersistentConfig bool) *ConfigFlags {
 		// given 25 groups (our groups + a few custom resources) with one-ish version each, discovery needs to make 50 requests
 		// double it just so we don't end up here again for a while.  This config is only used for discovery.
 		discoveryBurst: 100,
+		CacheTTL:       durationPtr(1 * time.Hour),


In my opinion the default can even be a full day. CRDs don't change that often and if a GVK can't be found in the cache, the cache is rendered invalid and repopulated anyway.

I think, let cache-ttl default to 1 hour and we also have a configuration option for the same. 1 day looks very long duration.

jonnylangefeld

This looks good to me. The default can be increased though IMO.

MadhavJivrajani · 2022-01-07T06:10:52Z

/ok-to-test

mk46 · 2022-01-07T07:29:59Z

/test pull-kubernetes-e2e-kind-ipv6

mk46 · 2022-01-11T06:08:56Z

/assign @soltysh

mk46 · 2022-01-27T05:41:46Z

@KnVerey @soltysh PTAL

eddiezane · 2022-02-10T19:05:43Z

Hi @mk46 - please edit the top level comment to follow the initial pull request template. Specifically we need the release note block.

/assign

soltysh · 2022-03-03T12:01:33Z

@mk46 when you get this updated please ping me on slack to get my immediate attention on this PR :) Thank you!

soltysh

/lgtm
/approve

k8s-ci-robot · 2022-03-03T16:06:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonnylangefeld, mk46, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~staging/src/k8s.io/cli-runtime/OWNERS~~ [soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

eddiezane · 2022-03-03T22:29:01Z

I think the flake is this #108010

/retest

k8s-triage-robot · 2022-03-04T02:06:53Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

sali2801 · 2022-09-18T15:42:31Z

is this PR only for diskcache? or even for memCache?

Increase default value of cache TTL to 6 hours.

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 20, 2021

k8s-ci-robot requested review from KnVerey and soltysh December 20, 2021 14:01

jonnylangefeld reviewed Jan 2, 2022

View reviewed changes

jonnylangefeld approved these changes Jan 2, 2022

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 7, 2022

k8s-ci-robot assigned soltysh Jan 11, 2022

k8s-ci-robot assigned eddiezane Feb 10, 2022

k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 17, 2022

k8s-ci-robot removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 3, 2022

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 3, 2022

mk46 force-pushed the ttl branch from 5489bd7 to 163ef10 Compare March 3, 2022 13:13

k8s-ci-robot added the area/dependency Issues or PRs related to dependency changes label Mar 3, 2022

mk46 force-pushed the ttl branch from 163ef10 to 1241f15 Compare March 3, 2022 15:44

k8s-ci-robot removed the do-not-merge/contains-merge-commits Indicates a PR which contains merge commits. label Mar 3, 2022

Increase default value of cache TTL to 6 hours.

94f7f92

mk46 force-pushed the ttl branch from 1241f15 to 94f7f92 Compare March 3, 2022 16:03

soltysh reviewed Mar 3, 2022

View reviewed changes

soltysh approved these changes Mar 3, 2022

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 3, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 3, 2022

mk46 changed the title ~~Add cache-ttl flag to increase ttl of discovery cache~~ Increase default value of cache TTL to 6 hours. Mar 3, 2022

k8s-ci-robot merged commit 04c8807 into kubernetes:master Mar 4, 2022

k8s-ci-robot added this to the v1.24 milestone Mar 4, 2022

github-actions bot mentioned this pull request Mar 22, 2022

Week Ending March 6, 2022 dev-obs/actus#394

Open

superbrothers mentioned this pull request May 9, 2022

Fix discovery cache TTL to 6 hours #109896

Merged

negz mentioned this pull request Jun 14, 2022

KEP-3352: Aggregated Discovery KEP to Alpha kubernetes/enhancements#3364

Merged

jonnylangefeld mentioned this pull request Dec 7, 2022

Lot's of discovery cache throttling with many CRDs kubernetes-sigs/controller-runtime#2084

Closed

YitzyD pushed a commit to YitzyD/kubernetes that referenced this pull request Mar 1, 2023

Merge pull request kubernetes#107141 from mk46/ttl

849624f

Increase default value of cache TTL to 6 hours.

YitzyD pushed a commit to YitzyD/kubernetes that referenced this pull request Mar 1, 2023

Merge pull request kubernetes#107141 from mk46/ttl

746bb7d

Increase default value of cache TTL to 6 hours.

jacobtomlinson mentioned this pull request Dec 15, 2023

Cache api-resources for longer kr8s-org/kr8s#255

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase default value of cache TTL to 6 hours. #107141

Increase default value of cache TTL to 6 hours. #107141

mk46 commented Dec 20, 2021 •

edited by soltysh

Loading

k8s-ci-robot commented Dec 20, 2021

mk46 commented Dec 20, 2021

jonnylangefeld Jan 2, 2022

mk46 Jan 7, 2022

jonnylangefeld left a comment

MadhavJivrajani commented Jan 7, 2022

mk46 commented Jan 7, 2022

mk46 commented Jan 11, 2022

mk46 commented Jan 27, 2022

eddiezane commented Feb 10, 2022

soltysh commented Mar 3, 2022

soltysh left a comment

k8s-ci-robot commented Mar 3, 2022

eddiezane commented Mar 3, 2022

k8s-triage-robot commented Mar 4, 2022

sali2801 commented Sep 18, 2022 •

edited

Loading

Increase default value of cache TTL to 6 hours. #107141

Increase default value of cache TTL to 6 hours. #107141

Conversation

mk46 commented Dec 20, 2021 • edited by soltysh Loading

Special notes for your reviewer:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Dec 20, 2021

mk46 commented Dec 20, 2021

jonnylangefeld Jan 2, 2022

Choose a reason for hiding this comment

mk46 Jan 7, 2022

Choose a reason for hiding this comment

jonnylangefeld left a comment

Choose a reason for hiding this comment

MadhavJivrajani commented Jan 7, 2022

mk46 commented Jan 7, 2022

mk46 commented Jan 11, 2022

mk46 commented Jan 27, 2022

eddiezane commented Feb 10, 2022

soltysh commented Mar 3, 2022

soltysh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Mar 3, 2022

eddiezane commented Mar 3, 2022

k8s-triage-robot commented Mar 4, 2022

sali2801 commented Sep 18, 2022 • edited Loading

mk46 commented Dec 20, 2021 •

edited by soltysh

Loading

sali2801 commented Sep 18, 2022 •

edited

Loading