{"action":"created","comment":{"url":"https://api.github.com/repos/square/keywhiz-fs/pulls/comments/68857734","id":68857734,"diff_hunk":"@@ -89,96 +89,75 @@ func (c *Cache) Clear() {\n // Secret retrieves a Secret by name from cache or a server.\n //\n // Cache logic:\n-//  * If cache hit and very recent: return cache entry\n-//  * Ask backend w/ timeout\n-//  * If backend returns fast: update cache, return\n-//  * If backend returns deleted: set delayed deletion, return data from cache\n-//  * If timeout_backend_deadline AND cache hit: return cache entry, background update cache when\n-//    backend returns\n-//  * If timeout_max_wait: log error and pretend file doesn't exist\n+//  1. Check cache for secret.\n+//\t\t\t* If entry is fresh, return cache entry.\n+//\t\t\t* If entry is not fresh, call backend.\n+//  2. Ask backend for secret (with timeout).\n+//\t\t\t* If backend returns success: update cache, return.\n+//\t\t\t* If backend returns deleted: set delayed deletion, return data from cache.\n+//  3. If timeout backend deadline hit return whatever we have.\n func (c *Cache) Secret(name string) (*Secret, bool) {\n-\tfailureDeadline := time.After(c.timeouts.MaxWait)\n-\tvar backendDeadline <-chan time.Time // inactive, until backend request starts\n+\t// Perform cache lookup first\n+\tcacheResult := c.cacheSecret(name)\n \n-\tvar cachedSecret *Secret\n-\tresultFromCache := func() (*Secret, bool) {\n-\t\tsuccess := cachedSecret != nil\n-\t\treturn cachedSecret, success\n-\t}\n+\tvar secret *Secret\n+\tvar success bool\n \n-\tcacheDone := c.cacheSecret(name)\n-\tvar backendDone chan secretResult\n+\tif cacheResult != nil {\n+\t\tsecret = &cacheResult.Secret\n+\t\tsuccess = true\n+\t}\n \n-\tfor {\n-\t\tselect {\n-\t\tcase s := <-backendDone:\n-\t\t\tbackendDone = nil\n-\t\t\tif s.err == nil {\n-\t\t\t\t// Always return successful value from backend\n-\t\t\t\treturn s.secret, true\n-\t\t\t}\n-\t\t\tif _, ok := s.err.(SecretDeleted); ok {\n-\t\t\t\tc.secretMap.Delete(name)\n-\t\t\t}\n-\t\t\t// Backend failed and cache lookup already finished\n-\t\t\tif cacheDone == nil {\n-\t\t\t\treturn resultFromCache()\n-\t\t\t}\n-\t\tcase s := <-cacheDone:\n-\t\t\tcacheDone = nil\n-\t\t\tif s != nil {\n-\t\t\t\tcachedSecret = &s.Secret\n+\t// If cache succeeded, and entry is very recent, return cache result\n+\tif success && time.Since(cacheResult.Time) < c.timeouts.Fresh {\n+\t\treturn &cacheResult.Secret, success\n+\t}\n \n-\t\t\t\t// If cache entry very recent, return cache result\n-\t\t\t\tif time.Since(s.Time) < c.timeouts.Fresh {\n-\t\t\t\t\treturn resultFromCache()\n-\t\t\t\t}\n-\t\t\t}\n+\tbackendDeadline := time.After(c.timeouts.BackendDeadline)\n+\tbackendDone := c.backendSecret(name)\n \n-\t\t\t// Start backend request and wait until optimistic deadline\n-\t\t\tbackendDone = c.backendSecret(name)\n-\t\t\tbackendDeadline = time.After(c.timeouts.BackendDeadline)\n-\t\tcase <-backendDeadline:\n-\t\t\tif cachedSecret != nil {\n-\t\t\t\treturn cachedSecret, true\n-\t\t\t}\n-\t\tcase <-failureDeadline:\n-\t\t\tc.Errorf(\"Cache and backend timeout: %v\", name)\n-\t\t\treturn nil, false\n+\tselect {\n+\tcase s := <-backendDone:\n+\t\tif s.err == nil {\n+\t\t\tsecret = s.secret\n+\t\t\tsuccess = true\n \t\t}\n+\t\tif _, ok := s.err.(SecretDeleted); ok {\n+\t\t\tc.secretMap.Delete(name)\n+\t\t}\n+\tcase <-backendDeadline:\n+\t\tc.Errorf(\"Backend timeout on secret fetch for '%s'\", name)\n \t}\n+\n+\treturn secret, success\n }\n \n // SecretList returns a listing of Secrets from cache or a server.\n //\n // Cache logic:\n-//  * Ask backend w/ timeout\n-//  * If backend returns fast: update cache, return\n-//  * If timeout_backend_deadline: return cache entries, background update cache when\n-//    backend returns\n-//  * If timeout_max_wait: log error and pretend no files\n+//  * Fetch cache secret list.","path":"cache.go","position":105,"original_position":105,"commit_id":"9d9c74c184e5757c6b290a4b15cd134991b2234b","original_commit_id":"9d9c74c184e5757c6b290a4b15cd134991b2234b","user":{"login":"alokmenghrajani","id":441307,"avatar_url":"https://avatars.githubusercontent.com/u/441307?v=3","gravatar_id":"","url":"https://api.github.com/users/alokmenghrajani","html_url":"https://github.com/alokmenghrajani","followers_url":"https://api.github.com/users/alokmenghrajani/followers","following_url":"https://api.github.com/users/alokmenghrajani/following{/other_user}","gists_url":"https://api.github.com/users/alokmenghrajani/gists{/gist_id}","starred_url":"https://api.github.com/users/alokmenghrajani/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alokmenghrajani/subscriptions","organizations_url":"https://api.github.com/users/alokmenghrajani/orgs","repos_url":"https://api.github.com/users/alokmenghrajani/repos","events_url":"https://api.github.com/users/alokmenghrajani/events{/privacy}","received_events_url":"https://api.github.com/users/alokmenghrajani/received_events","type":"User","site_admin":false},"body":"I feel it's more clear in this case to remove this line. The fetch is implicit in `return cache entries` below.","created_at":"2016-06-28T22:44:52Z","updated_at":"2016-06-28T22:44:52Z","html_url":"https://github.com/square/keywhiz-fs/pull/68#discussion_r68857734","pull_request_url":"https://api.github.com/repos/square/keywhiz-fs/pulls/68","_links":{"self":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/comments/68857734"},"html":{"href":"https://github.com/square/keywhiz-fs/pull/68#discussion_r68857734"},"pull_request":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/68"}}},"pull_request":{"url":"https://api.github.com/repos/square/keywhiz-fs/pulls/68","id":75562576,"html_url":"https://github.com/square/keywhiz-fs/pull/68","diff_url":"https://github.com/square/keywhiz-fs/pull/68.diff","patch_url":"https://github.com/square/keywhiz-fs/pull/68.patch","issue_url":"https://api.github.com/repos/square/keywhiz-fs/issues/68","number":68,"state":"open","locked":false,"title":"Simplify caching logic","user":{"login":"csstaub","id":639883,"avatar_url":"https://avatars.githubusercontent.com/u/639883?v=3","gravatar_id":"","url":"https://api.github.com/users/csstaub","html_url":"https://github.com/csstaub","followers_url":"https://api.github.com/users/csstaub/followers","following_url":"https://api.github.com/users/csstaub/following{/other_user}","gists_url":"https://api.github.com/users/csstaub/gists{/gist_id}","starred_url":"https://api.github.com/users/csstaub/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/csstaub/subscriptions","organizations_url":"https://api.github.com/users/csstaub/orgs","repos_url":"https://api.github.com/users/csstaub/repos","events_url":"https://api.github.com/users/csstaub/events{/privacy}","received_events_url":"https://api.github.com/users/csstaub/received_events","type":"User","site_admin":false},"body":"r: @alokmenghrajani @mcpherrinm @worldwise001 ","created_at":"2016-06-28T22:40:48Z","updated_at":"2016-06-28T22:44:52Z","closed_at":null,"merged_at":null,"merge_commit_sha":"041848d77c56c393dfe4d36900d47c03d62580a4","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/square/keywhiz-fs/pulls/68/commits","review_comments_url":"https://api.github.com/repos/square/keywhiz-fs/pulls/68/comments","review_comment_url":"https://api.github.com/repos/square/keywhiz-fs/pulls/comments{/number}","comments_url":"https://api.github.com/repos/square/keywhiz-fs/issues/68/comments","statuses_url":"https://api.github.com/repos/square/keywhiz-fs/statuses/9d9c74c184e5757c6b290a4b15cd134991b2234b","head":{"label":"square:cs/simplify","ref":"cs/simplify","sha":"9d9c74c184e5757c6b290a4b15cd134991b2234b","user":{"login":"square","id":82592,"avatar_url":"https://avatars.githubusercontent.com/u/82592?v=3","gravatar_id":"","url":"https://api.github.com/users/square","html_url":"https://github.com/square","followers_url":"https://api.github.com/users/square/followers","following_url":"https://api.github.com/users/square/following{/other_user}","gists_url":"https://api.github.com/users/square/gists{/gist_id}","starred_url":"https://api.github.com/users/square/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/square/subscriptions","organizations_url":"https://api.github.com/users/square/orgs","repos_url":"https://api.github.com/users/square/repos","events_url":"https://api.github.com/users/square/events{/privacy}","received_events_url":"https://api.github.com/users/square/received_events","type":"Organization","site_admin":false},"repo":{"id":32866443,"name":"keywhiz-fs","full_name":"square/keywhiz-fs","owner":{"login":"square","id":82592,"avatar_url":"https://avatars.githubusercontent.com/u/82592?v=3","gravatar_id":"","url":"https://api.github.com/users/square","html_url":"https://github.com/square","followers_url":"https://api.github.com/users/square/followers","following_url":"https://api.github.com/users/square/following{/other_user}","gists_url":"https://api.github.com/users/square/gists{/gist_id}","starred_url":"https://api.github.com/users/square/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/square/subscriptions","organizations_url":"https://api.github.com/users/square/orgs","repos_url":"https://api.github.com/users/square/repos","events_url":"https://api.github.com/users/square/events{/privacy}","received_events_url":"https://api.github.com/users/square/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/square/keywhiz-fs","description":"A file-system client for Keywhiz","fork":false,"url":"https://api.github.com/repos/square/keywhiz-fs","forks_url":"https://api.github.com/repos/square/keywhiz-fs/forks","keys_url":"https://api.github.com/repos/square/keywhiz-fs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/square/keywhiz-fs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/square/keywhiz-fs/teams","hooks_url":"https://api.github.com/repos/square/keywhiz-fs/hooks","issue_events_url":"https://api.github.com/repos/square/keywhiz-fs/issues/events{/number}","events_url":"https://api.github.com/repos/square/keywhiz-fs/events","assignees_url":"https://api.github.com/repos/square/keywhiz-fs/assignees{/user}","branches_url":"https://api.github.com/repos/square/keywhiz-fs/branches{/branch}","tags_url":"https://api.github.com/repos/square/keywhiz-fs/tags","blobs_url":"https://api.github.com/repos/square/keywhiz-fs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/square/keywhiz-fs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/square/keywhiz-fs/git/refs{/sha}","trees_url":"https://api.github.com/repos/square/keywhiz-fs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/square/keywhiz-fs/statuses/{sha}","languages_url":"https://api.github.com/repos/square/keywhiz-fs/languages","stargazers_url":"https://api.github.com/repos/square/keywhiz-fs/stargazers","contributors_url":"https://api.github.com/repos/square/keywhiz-fs/contributors","subscribers_url":"https://api.github.com/repos/square/keywhiz-fs/subscribers","subscription_url":"https://api.github.com/repos/square/keywhiz-fs/subscription","commits_url":"https://api.github.com/repos/square/keywhiz-fs/commits{/sha}","git_commits_url":"https://api.github.com/repos/square/keywhiz-fs/git/commits{/sha}","comments_url":"https://api.github.com/repos/square/keywhiz-fs/comments{/number}","issue_comment_url":"https://api.github.com/repos/square/keywhiz-fs/issues/comments{/number}","contents_url":"https://api.github.com/repos/square/keywhiz-fs/contents/{+path}","compare_url":"https://api.github.com/repos/square/keywhiz-fs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/square/keywhiz-fs/merges","archive_url":"https://api.github.com/repos/square/keywhiz-fs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/square/keywhiz-fs/downloads","issues_url":"https://api.github.com/repos/square/keywhiz-fs/issues{/number}","pulls_url":"https://api.github.com/repos/square/keywhiz-fs/pulls{/number}","milestones_url":"https://api.github.com/repos/square/keywhiz-fs/milestones{/number}","notifications_url":"https://api.github.com/repos/square/keywhiz-fs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/square/keywhiz-fs/labels{/name}","releases_url":"https://api.github.com/repos/square/keywhiz-fs/releases{/id}","deployments_url":"https://api.github.com/repos/square/keywhiz-fs/deployments","created_at":"2015-03-25T13:31:56Z","updated_at":"2016-06-06T06:15:44Z","pushed_at":"2016-06-28T22:40:48Z","git_url":"git://github.com/square/keywhiz-fs.git","ssh_url":"git@github.com:square/keywhiz-fs.git","clone_url":"https://github.com/square/keywhiz-fs.git","svn_url":"https://github.com/square/keywhiz-fs","homepage":"https://square.github.io/keywhiz/","size":1091,"stargazers_count":97,"watchers_count":97,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":17,"mirror_url":null,"open_issues_count":6,"forks":17,"open_issues":6,"watchers":97,"default_branch":"master"}},"base":{"label":"square:master","ref":"master","sha":"cac87fbdcff98b999e55e5b160a7f92a5c2330e7","user":{"login":"square","id":82592,"avatar_url":"https://avatars.githubusercontent.com/u/82592?v=3","gravatar_id":"","url":"https://api.github.com/users/square","html_url":"https://github.com/square","followers_url":"https://api.github.com/users/square/followers","following_url":"https://api.github.com/users/square/following{/other_user}","gists_url":"https://api.github.com/users/square/gists{/gist_id}","starred_url":"https://api.github.com/users/square/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/square/subscriptions","organizations_url":"https://api.github.com/users/square/orgs","repos_url":"https://api.github.com/users/square/repos","events_url":"https://api.github.com/users/square/events{/privacy}","received_events_url":"https://api.github.com/users/square/received_events","type":"Organization","site_admin":false},"repo":{"id":32866443,"name":"keywhiz-fs","full_name":"square/keywhiz-fs","owner":{"login":"square","id":82592,"avatar_url":"https://avatars.githubusercontent.com/u/82592?v=3","gravatar_id":"","url":"https://api.github.com/users/square","html_url":"https://github.com/square","followers_url":"https://api.github.com/users/square/followers","following_url":"https://api.github.com/users/square/following{/other_user}","gists_url":"https://api.github.com/users/square/gists{/gist_id}","starred_url":"https://api.github.com/users/square/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/square/subscriptions","organizations_url":"https://api.github.com/users/square/orgs","repos_url":"https://api.github.com/users/square/repos","events_url":"https://api.github.com/users/square/events{/privacy}","received_events_url":"https://api.github.com/users/square/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/square/keywhiz-fs","description":"A file-system client for Keywhiz","fork":false,"url":"https://api.github.com/repos/square/keywhiz-fs","forks_url":"https://api.github.com/repos/square/keywhiz-fs/forks","keys_url":"https://api.github.com/repos/square/keywhiz-fs/keys{/key_id}","collaborators_url":"https://api.github.com/repos/square/keywhiz-fs/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/square/keywhiz-fs/teams","hooks_url":"https://api.github.com/repos/square/keywhiz-fs/hooks","issue_events_url":"https://api.github.com/repos/square/keywhiz-fs/issues/events{/number}","events_url":"https://api.github.com/repos/square/keywhiz-fs/events","assignees_url":"https://api.github.com/repos/square/keywhiz-fs/assignees{/user}","branches_url":"https://api.github.com/repos/square/keywhiz-fs/branches{/branch}","tags_url":"https://api.github.com/repos/square/keywhiz-fs/tags","blobs_url":"https://api.github.com/repos/square/keywhiz-fs/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/square/keywhiz-fs/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/square/keywhiz-fs/git/refs{/sha}","trees_url":"https://api.github.com/repos/square/keywhiz-fs/git/trees{/sha}","statuses_url":"https://api.github.com/repos/square/keywhiz-fs/statuses/{sha}","languages_url":"https://api.github.com/repos/square/keywhiz-fs/languages","stargazers_url":"https://api.github.com/repos/square/keywhiz-fs/stargazers","contributors_url":"https://api.github.com/repos/square/keywhiz-fs/contributors","subscribers_url":"https://api.github.com/repos/square/keywhiz-fs/subscribers","subscription_url":"https://api.github.com/repos/square/keywhiz-fs/subscription","commits_url":"https://api.github.com/repos/square/keywhiz-fs/commits{/sha}","git_commits_url":"https://api.github.com/repos/square/keywhiz-fs/git/commits{/sha}","comments_url":"https://api.github.com/repos/square/keywhiz-fs/comments{/number}","issue_comment_url":"https://api.github.com/repos/square/keywhiz-fs/issues/comments{/number}","contents_url":"https://api.github.com/repos/square/keywhiz-fs/contents/{+path}","compare_url":"https://api.github.com/repos/square/keywhiz-fs/compare/{base}...{head}","merges_url":"https://api.github.com/repos/square/keywhiz-fs/merges","archive_url":"https://api.github.com/repos/square/keywhiz-fs/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/square/keywhiz-fs/downloads","issues_url":"https://api.github.com/repos/square/keywhiz-fs/issues{/number}","pulls_url":"https://api.github.com/repos/square/keywhiz-fs/pulls{/number}","milestones_url":"https://api.github.com/repos/square/keywhiz-fs/milestones{/number}","notifications_url":"https://api.github.com/repos/square/keywhiz-fs/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/square/keywhiz-fs/labels{/name}","releases_url":"https://api.github.com/repos/square/keywhiz-fs/releases{/id}","deployments_url":"https://api.github.com/repos/square/keywhiz-fs/deployments","created_at":"2015-03-25T13:31:56Z","updated_at":"2016-06-06T06:15:44Z","pushed_at":"2016-06-28T22:40:48Z","git_url":"git://github.com/square/keywhiz-fs.git","ssh_url":"git@github.com:square/keywhiz-fs.git","clone_url":"https://github.com/square/keywhiz-fs.git","svn_url":"https://github.com/square/keywhiz-fs","homepage":"https://square.github.io/keywhiz/","size":1091,"stargazers_count":97,"watchers_count":97,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":17,"mirror_url":null,"open_issues_count":6,"forks":17,"open_issues":6,"watchers":97,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/68"},"html":{"href":"https://github.com/square/keywhiz-fs/pull/68"},"issue":{"href":"https://api.github.com/repos/square/keywhiz-fs/issues/68"},"comments":{"href":"https://api.github.com/repos/square/keywhiz-fs/issues/68/comments"},"review_comments":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/68/comments"},"review_comment":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/square/keywhiz-fs/pulls/68/commits"},"statuses":{"href":"https://api.github.com/repos/square/keywhiz-fs/statuses/9d9c74c184e5757c6b290a4b15cd134991b2234b"}}}}
{"actor":{"display_login":"alokmenghrajani"}}