{"action":"created","issue":{"url":"https://api.github.com/repos/knative/serving/issues/1960","repository_url":"https://api.github.com/repos/knative/serving","labels_url":"https://api.github.com/repos/knative/serving/issues/1960/labels{/name}","comments_url":"https://api.github.com/repos/knative/serving/issues/1960/comments","events_url":"https://api.github.com/repos/knative/serving/issues/1960/events","html_url":"https://github.com/knative/serving/issues/1960","id":354767370,"node_id":"MDU6SXNzdWUzNTQ3NjczNzA=","number":1960,"title":"Revision GC","user":{"login":"mattmoor","id":2442466,"node_id":"MDQ6VXNlcjI0NDI0NjY=","avatar_url":"https://avatars3.githubusercontent.com/u/2442466?v=4","gravatar_id":"","url":"https://api.github.com/users/mattmoor","html_url":"https://github.com/mattmoor","followers_url":"https://api.github.com/users/mattmoor/followers","following_url":"https://api.github.com/users/mattmoor/following{/other_user}","gists_url":"https://api.github.com/users/mattmoor/gists{/gist_id}","starred_url":"https://api.github.com/users/mattmoor/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mattmoor/subscriptions","organizations_url":"https://api.github.com/users/mattmoor/orgs","repos_url":"https://api.github.com/users/mattmoor/repos","events_url":"https://api.github.com/users/mattmoor/events{/privacy}","received_events_url":"https://api.github.com/users/mattmoor/received_events","type":"User","site_admin":false},"labels":[{"id":825408057,"node_id":"MDU6TGFiZWw4MjU0MDgwNTc=","url":"https://api.github.com/repos/knative/serving/labels/area/API","name":"area/API","color":"0052cc","default":false},{"id":917112268,"node_id":"MDU6TGFiZWw5MTcxMTIyNjg=","url":"https://api.github.com/repos/knative/serving/labels/kind/cleanup","name":"kind/cleanup","color":"fef2c0","default":false},{"id":816839988,"node_id":"MDU6TGFiZWw4MTY4Mzk5ODg=","url":"https://api.github.com/repos/knative/serving/labels/kind/feature","name":"kind/feature","color":"1d76db","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2018-08-28T14:50:10Z","updated_at":"2018-09-14T18:34:23Z","closed_at":null,"author_association":"MEMBER","body":"<!--\r\n/area API\r\n/kind cleanup\r\n/kind feature\r\n-->\r\n\r\n*Using this to replace https://github.com/knative/serving/issues/70 and clean up this history*\r\n\r\n## Expected Behavior\r\nWe should retain a finite set of reachable `Revisions`.\r\n\r\n## Actual Behavior\r\nWe retain all `Revisions`.\r\n\r\n### In detail\r\n\r\nToday as Revisions fall out of use they are scaled to zero *pods* reclaiming key resources.  However, inactive Revisions still have several Kubernetes resources (which in turn have their own) that have a non-zero cost.\r\n * We scale the Deployment down to zero (likely still has a ReplicaSet), and these are still seen by their respective controllers during resyncs.\r\n * We keep the Kubernetes Service (still holds a ClusterIP).\r\n * We keep the KPA (still keeps some in memory state for metrics)\r\n\r\nIn the (near) future:\r\n * We will keep around a `caching.internal.knative.dev/v1alpha1/Image`, which at a minimum eat space in `etcd`, and may eat space on Nodes (e.g. github.com/mattmoor/warm-image) or other caching infrastructure.\r\n\r\n### Gotchas\r\n\r\nWe have largely tried to keep sane layering in `knative/serving` and the Revision is largely ignorant of Configuration, which is largely ignorant of Route, which are both largely ignorant of Service.\r\n\r\n**So what owns the GC?**\r\n\r\nI think the natural place for this is the Configuration controller, since it is the Revision collection and resources like Route won't necessarily maintain their `1:N` relationship in perpetuity (so multiple routes may reference a Revision).\r\n\r\nThis is fine when the Route simply references the Configuration (mutable), but not when it references Revisions (immutable).  My Route can pin an ancient Revision, and we need to keep it around.\r\n\r\n**We need a signal that a Revision is \"pinned\" and shouldn't be GC'd**\r\n\r\nFirst, a naive annotation:\r\n```yaml\r\nkind: Revision\r\nmetadata:\r\n  name: foo\r\n  namespace: bar\r\n  annotations:\r\n    revision.knative.dev/pinned: true\r\n```\r\n\r\nHowever, this has two problems:\r\n1. Routes need to remove these annotations when a Revision is unpinned (and K8s lacks support for multi-resource transactions, creating a race)\r\n1. Routes may not remain `1:N`, which creates a future reference counting problem.\r\n\r\n\r\nSecond, a less naive annotation:\r\n```yaml\r\nkind: Revision\r\nmetadata:\r\n  name: foo\r\n  namespace: bar\r\n  annotations:\r\n    # Or pinned until?\r\n    revision.knative.dev/lastPinned: <timestamp>\r\n```\r\n\r\nThe Revision controller would only put this on directly references Revisions, and would periodically update the timestamp via resyncs.  This solves both of the above problems:\r\n1. As Routes unpin Revisions, the annotation starts to age out.\r\n1. Multiple Routes can bump the same Revision's timestamp, until they do not.\r\n\r\n\r\nI expect that we would expose knobs to control certain dimensions of this via operator configmaps, but my thinking hasn't gotten that far, and I'd like to see what folks think about how we manage pinned revisions.\r\n\r\ncc @dprotaso @tcnghia @grantr @vaikas-google @evankanderson @mikehelmick for thoughts."},"comment":{"url":"https://api.github.com/repos/knative/serving/issues/comments/421447179","html_url":"https://github.com/knative/serving/issues/1960#issuecomment-421447179","issue_url":"https://api.github.com/repos/knative/serving/issues/1960","id":421447179,"node_id":"MDEyOklzc3VlQ29tbWVudDQyMTQ0NzE3OQ==","user":{"login":"mattmoor","id":2442466,"node_id":"MDQ6VXNlcjI0NDI0NjY=","avatar_url":"https://avatars3.githubusercontent.com/u/2442466?v=4","gravatar_id":"","url":"https://api.github.com/users/mattmoor","html_url":"https://github.com/mattmoor","followers_url":"https://api.github.com/users/mattmoor/followers","following_url":"https://api.github.com/users/mattmoor/following{/other_user}","gists_url":"https://api.github.com/users/mattmoor/gists{/gist_id}","starred_url":"https://api.github.com/users/mattmoor/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mattmoor/subscriptions","organizations_url":"https://api.github.com/users/mattmoor/orgs","repos_url":"https://api.github.com/users/mattmoor/repos","events_url":"https://api.github.com/users/mattmoor/events{/privacy}","received_events_url":"https://api.github.com/users/mattmoor/received_events","type":"User","site_admin":false},"created_at":"2018-09-14T18:34:23Z","updated_at":"2018-09-14T18:34:23Z","author_association":"MEMBER","body":"Yeah, I think I like this for the liveness model.  I think we can explore a number of retention controls around knowing what we need.  Thanks, @mikehelmick "}}
{"actor":{"display_login":"mattmoor"}}