{"action":"created","comment":{"url":"https://api.github.com/repos/influxdb/influxdb/pulls/comments/46595521","id":46595521,"diff_hunk":"@@ -209,117 +187,61 @@ func (c *Cache) Keys() []string {\n \treturn a\n }\n \n-func (c *Cache) KeyRange(min, max uint64) []string {\n-\tvar a []string\n-\tfor k, v := range c.store {\n-\t\tif v.contains(min, max) {\n-\t\t\ta = append(a, k)\n+// Values returns a copy of all values, deduped and sorted, for the given key.\n+func (c *Cache) Values(key string) Values {\n+\tvalues, needSort := func() (Values, bool) {\n+\t\tc.mu.RLock()\n+\t\tdefer c.mu.RUnlock()\n+\t\te := c.store[key]\n+\t\tif e == nil {\n+\t\t\treturn nil, false\n \t\t}\n-\t}\n-\tsort.Strings(a)\n-\treturn a\n-}\n \n-// Checkpoint returns the current checkpoint for the cache.\n-func (c *Cache) Checkpoint() uint64 {\n-\tc.mu.RLock()\n-\tdefer c.mu.RUnlock()\n-\treturn c.checkpoint\n-}\n-\n-func (c *Cache) CheckpointRange() (uint64, uint64) {\n-\tc.mu.RLock()\n-\tdefer c.mu.RUnlock()\n-\n-\tvar min, max uint64\n-\tfor _, v := range c.store {\n-\t\tfor checkpoint := range v.m {\n-\t\t\tif min == 0 || checkpoint < min {\n-\t\t\t\tmin = checkpoint\n-\t\t\t}\n-\t\t\tif max == 0 || checkpoint > max {\n-\t\t\t\tmax = checkpoint\n-\t\t\t}\n+\t\tif e.needSort {\n+\t\t\treturn nil, true\n \t\t}\n-\t}\n-\treturn min, max\n-}\n \n-// Evict forces the cache to evict.\n-func (c *Cache) Evict() {\n-\tc.mu.Lock()\n-\tdefer c.mu.Unlock()\n-\tc.evict()\n-}\n-\n-// Values returns a copy of all values, deduped and sorted as requested, for the given key.\n-func (c *Cache) Values(key string, ascending bool) Values {\n-\tvalues := func() Values {\n-\t\tc.mu.RLock()\n-\t\tdefer c.mu.RUnlock()\n-\t\te, ok := c.store[key]\n-\t\tif !ok {\n-\t\t\treturn nil\n-\t\t}\n-\t\treturn e.clone()\n+\t\treturn e.values[0:len(e.values)], false\n \t}()\n-\t// Now have copy, so perform dedupe and sort outside of lock, unblocking\n-\t// writes.\n \n-\tif values == nil {\n-\t\treturn nil\n-\t}\n-\treturn values.Deduplicate(ascending)\n-}\n+\t// the values in the entry require a sort, do so with a write lock so\n+\t// we can sort once and set everything in order\n+\tif needSort {","path":"tsdb/engine/tsm1/cache.go","position":345,"original_position":354,"commit_id":"4624fb2a78f51c0d55b2761d3070e15957ccc4ce","original_commit_id":"be4891c40b644400d18af562f3c846dbfb340e90","user":{"login":"pauldix","id":4331,"avatar_url":"https://avatars.githubusercontent.com/u/4331?v=3","gravatar_id":"","url":"https://api.github.com/users/pauldix","html_url":"https://github.com/pauldix","followers_url":"https://api.github.com/users/pauldix/followers","following_url":"https://api.github.com/users/pauldix/following{/other_user}","gists_url":"https://api.github.com/users/pauldix/gists{/gist_id}","starred_url":"https://api.github.com/users/pauldix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pauldix/subscriptions","organizations_url":"https://api.github.com/users/pauldix/orgs","repos_url":"https://api.github.com/users/pauldix/repos","events_url":"https://api.github.com/users/pauldix/events{/privacy}","received_events_url":"https://api.github.com/users/pauldix/received_events","type":"User","site_admin":false},"body":"At 192 I have the lock. I grab a subslice of the values while I have the lock and that's what I return. At line 200 when I check, I only care if the sublice that I already grabbed while under lock needs sorting.\r\n\r\nIf it does, then I grab the lock again, sort, grab a new subslice and return.\r\n\r\nIf writes come in between 192 and 200 that cause `e.values` to be unsorted, I don't care because those are all at the end of the slice, which aren't part of the subslice that I'll be returning.","created_at":"2015-12-03T19:07:03Z","updated_at":"2015-12-03T19:07:03Z","html_url":"https://github.com/influxdb/influxdb/pull/4966#discussion_r46595521","pull_request_url":"https://api.github.com/repos/influxdb/influxdb/pulls/4966","_links":{"self":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/comments/46595521"},"html":{"href":"https://github.com/influxdb/influxdb/pull/4966#discussion_r46595521"},"pull_request":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/4966"}}},"pull_request":{"url":"https://api.github.com/repos/influxdb/influxdb/pulls/4966","id":52533571,"html_url":"https://github.com/influxdb/influxdb/pull/4966","diff_url":"https://github.com/influxdb/influxdb/pull/4966.diff","patch_url":"https://github.com/influxdb/influxdb/pull/4966.patch","issue_url":"https://api.github.com/repos/influxdb/influxdb/issues/4966","number":4966,"state":"open","locked":false,"title":"Update TSM cache, compact, wal, encoding","user":{"login":"pauldix","id":4331,"avatar_url":"https://avatars.githubusercontent.com/u/4331?v=3","gravatar_id":"","url":"https://api.github.com/users/pauldix","html_url":"https://github.com/pauldix","followers_url":"https://api.github.com/users/pauldix/followers","following_url":"https://api.github.com/users/pauldix/following{/other_user}","gists_url":"https://api.github.com/users/pauldix/gists{/gist_id}","starred_url":"https://api.github.com/users/pauldix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pauldix/subscriptions","organizations_url":"https://api.github.com/users/pauldix/orgs","repos_url":"https://api.github.com/users/pauldix/repos","events_url":"https://api.github.com/users/pauldix/events{/privacy}","received_events_url":"https://api.github.com/users/pauldix/received_events","type":"User","site_admin":false},"body":"* Update cache to have a single slice of values for a key (removed checkpoints)\r\n* Changed compact.Plan to only worry about TSM files.\r\n* Updated Plan to not return an error since there was no case in which it would.\r\n* Update WAL to not keep stats since they're no longer needed.\r\n* Update engine to flush the Cache/WAL to a new TSM file when the min threshold is hit.\r\n* Split compact logic between TSM compacts and WAL/Cache writes.\r\n* Remove unnecessary merge iterator, wal segment iterator, and other no longer necessary stuff.\r\n* Remove the asending bool from the Dedupe method. Values should always be in ascending order. It's up to the cursor to iterate through values based on the direction. Giving the cursor responsibility makes it so we don't need to sort, dedupe or reallocate anything for different query orders.\r\n* Updated engine to use its locks to ensure writes and cache flushes don't cause a race.\r\n* Update all tests with new signatures. Removed a bunch of tests around TSM rewrites and WAL segment iteration that are no longer necessary.\r\n\r\nThis is mergeable and ready to go, but there are a few other things that still need to be done:\r\n- [ ] Update Cache cursor to combine snapshot caches and the hot cache\r\n- [x] We need a test for the `Compact` method that actually combines multiple TSM files (don't think this was there previously)\r\n- [ ] Update the `Plan` logic to have some tunable parameters around when TSM files are selected for compaction. For instance, only compacting files older than some given time. Then compacting full after no writes have been received for a while.\r\n- [ ] Add some logic for forcing a snapshot write if a shard hasn't received a write in some configurable amount of time.","created_at":"2015-12-03T13:27:02Z","updated_at":"2015-12-03T19:07:03Z","closed_at":null,"merged_at":null,"merge_commit_sha":"adc193974a7e6c97db6462ddbb4b6b0e15889905","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/influxdb/influxdb/pulls/4966/commits","review_comments_url":"https://api.github.com/repos/influxdb/influxdb/pulls/4966/comments","review_comment_url":"https://api.github.com/repos/influxdb/influxdb/pulls/comments{/number}","comments_url":"https://api.github.com/repos/influxdb/influxdb/issues/4966/comments","statuses_url":"https://api.github.com/repos/influxdb/influxdb/statuses/4624fb2a78f51c0d55b2761d3070e15957ccc4ce","head":{"label":"influxdb:pd-walcache-perf","ref":"pd-walcache-perf","sha":"4624fb2a78f51c0d55b2761d3070e15957ccc4ce","user":{"login":"influxdb","id":5713248,"avatar_url":"https://avatars.githubusercontent.com/u/5713248?v=3","gravatar_id":"","url":"https://api.github.com/users/influxdb","html_url":"https://github.com/influxdb","followers_url":"https://api.github.com/users/influxdb/followers","following_url":"https://api.github.com/users/influxdb/following{/other_user}","gists_url":"https://api.github.com/users/influxdb/gists{/gist_id}","starred_url":"https://api.github.com/users/influxdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/influxdb/subscriptions","organizations_url":"https://api.github.com/users/influxdb/orgs","repos_url":"https://api.github.com/users/influxdb/repos","events_url":"https://api.github.com/users/influxdb/events{/privacy}","received_events_url":"https://api.github.com/users/influxdb/received_events","type":"Organization","site_admin":false},"repo":{"id":13124802,"name":"influxdb","full_name":"influxdb/influxdb","owner":{"login":"influxdb","id":5713248,"avatar_url":"https://avatars.githubusercontent.com/u/5713248?v=3","gravatar_id":"","url":"https://api.github.com/users/influxdb","html_url":"https://github.com/influxdb","followers_url":"https://api.github.com/users/influxdb/followers","following_url":"https://api.github.com/users/influxdb/following{/other_user}","gists_url":"https://api.github.com/users/influxdb/gists{/gist_id}","starred_url":"https://api.github.com/users/influxdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/influxdb/subscriptions","organizations_url":"https://api.github.com/users/influxdb/orgs","repos_url":"https://api.github.com/users/influxdb/repos","events_url":"https://api.github.com/users/influxdb/events{/privacy}","received_events_url":"https://api.github.com/users/influxdb/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/influxdb/influxdb","description":"Scalable datastore for metrics, events, and real-time analytics","fork":false,"url":"https://api.github.com/repos/influxdb/influxdb","forks_url":"https://api.github.com/repos/influxdb/influxdb/forks","keys_url":"https://api.github.com/repos/influxdb/influxdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/influxdb/influxdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/influxdb/influxdb/teams","hooks_url":"https://api.github.com/repos/influxdb/influxdb/hooks","issue_events_url":"https://api.github.com/repos/influxdb/influxdb/issues/events{/number}","events_url":"https://api.github.com/repos/influxdb/influxdb/events","assignees_url":"https://api.github.com/repos/influxdb/influxdb/assignees{/user}","branches_url":"https://api.github.com/repos/influxdb/influxdb/branches{/branch}","tags_url":"https://api.github.com/repos/influxdb/influxdb/tags","blobs_url":"https://api.github.com/repos/influxdb/influxdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/influxdb/influxdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/influxdb/influxdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/influxdb/influxdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/influxdb/influxdb/statuses/{sha}","languages_url":"https://api.github.com/repos/influxdb/influxdb/languages","stargazers_url":"https://api.github.com/repos/influxdb/influxdb/stargazers","contributors_url":"https://api.github.com/repos/influxdb/influxdb/contributors","subscribers_url":"https://api.github.com/repos/influxdb/influxdb/subscribers","subscription_url":"https://api.github.com/repos/influxdb/influxdb/subscription","commits_url":"https://api.github.com/repos/influxdb/influxdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/influxdb/influxdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/influxdb/influxdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/influxdb/influxdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/influxdb/influxdb/contents/{+path}","compare_url":"https://api.github.com/repos/influxdb/influxdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/influxdb/influxdb/merges","archive_url":"https://api.github.com/repos/influxdb/influxdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/influxdb/influxdb/downloads","issues_url":"https://api.github.com/repos/influxdb/influxdb/issues{/number}","pulls_url":"https://api.github.com/repos/influxdb/influxdb/pulls{/number}","milestones_url":"https://api.github.com/repos/influxdb/influxdb/milestones{/number}","notifications_url":"https://api.github.com/repos/influxdb/influxdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/influxdb/influxdb/labels{/name}","releases_url":"https://api.github.com/repos/influxdb/influxdb/releases{/id}","created_at":"2013-09-26T14:31:10Z","updated_at":"2015-12-03T17:10:06Z","pushed_at":"2015-12-03T19:04:13Z","git_url":"git://github.com/influxdb/influxdb.git","ssh_url":"git@github.com:influxdb/influxdb.git","clone_url":"https://github.com/influxdb/influxdb.git","svn_url":"https://github.com/influxdb/influxdb","homepage":"http://influxdb.com","size":38767,"stargazers_count":6867,"watchers_count":6867,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":831,"mirror_url":null,"open_issues_count":502,"forks":831,"open_issues":502,"watchers":6867,"default_branch":"master"}},"base":{"label":"influxdb:master","ref":"master","sha":"d85f3c3dc78324c3e9543dd3d8ec9ab762330091","user":{"login":"influxdb","id":5713248,"avatar_url":"https://avatars.githubusercontent.com/u/5713248?v=3","gravatar_id":"","url":"https://api.github.com/users/influxdb","html_url":"https://github.com/influxdb","followers_url":"https://api.github.com/users/influxdb/followers","following_url":"https://api.github.com/users/influxdb/following{/other_user}","gists_url":"https://api.github.com/users/influxdb/gists{/gist_id}","starred_url":"https://api.github.com/users/influxdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/influxdb/subscriptions","organizations_url":"https://api.github.com/users/influxdb/orgs","repos_url":"https://api.github.com/users/influxdb/repos","events_url":"https://api.github.com/users/influxdb/events{/privacy}","received_events_url":"https://api.github.com/users/influxdb/received_events","type":"Organization","site_admin":false},"repo":{"id":13124802,"name":"influxdb","full_name":"influxdb/influxdb","owner":{"login":"influxdb","id":5713248,"avatar_url":"https://avatars.githubusercontent.com/u/5713248?v=3","gravatar_id":"","url":"https://api.github.com/users/influxdb","html_url":"https://github.com/influxdb","followers_url":"https://api.github.com/users/influxdb/followers","following_url":"https://api.github.com/users/influxdb/following{/other_user}","gists_url":"https://api.github.com/users/influxdb/gists{/gist_id}","starred_url":"https://api.github.com/users/influxdb/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/influxdb/subscriptions","organizations_url":"https://api.github.com/users/influxdb/orgs","repos_url":"https://api.github.com/users/influxdb/repos","events_url":"https://api.github.com/users/influxdb/events{/privacy}","received_events_url":"https://api.github.com/users/influxdb/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/influxdb/influxdb","description":"Scalable datastore for metrics, events, and real-time analytics","fork":false,"url":"https://api.github.com/repos/influxdb/influxdb","forks_url":"https://api.github.com/repos/influxdb/influxdb/forks","keys_url":"https://api.github.com/repos/influxdb/influxdb/keys{/key_id}","collaborators_url":"https://api.github.com/repos/influxdb/influxdb/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/influxdb/influxdb/teams","hooks_url":"https://api.github.com/repos/influxdb/influxdb/hooks","issue_events_url":"https://api.github.com/repos/influxdb/influxdb/issues/events{/number}","events_url":"https://api.github.com/repos/influxdb/influxdb/events","assignees_url":"https://api.github.com/repos/influxdb/influxdb/assignees{/user}","branches_url":"https://api.github.com/repos/influxdb/influxdb/branches{/branch}","tags_url":"https://api.github.com/repos/influxdb/influxdb/tags","blobs_url":"https://api.github.com/repos/influxdb/influxdb/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/influxdb/influxdb/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/influxdb/influxdb/git/refs{/sha}","trees_url":"https://api.github.com/repos/influxdb/influxdb/git/trees{/sha}","statuses_url":"https://api.github.com/repos/influxdb/influxdb/statuses/{sha}","languages_url":"https://api.github.com/repos/influxdb/influxdb/languages","stargazers_url":"https://api.github.com/repos/influxdb/influxdb/stargazers","contributors_url":"https://api.github.com/repos/influxdb/influxdb/contributors","subscribers_url":"https://api.github.com/repos/influxdb/influxdb/subscribers","subscription_url":"https://api.github.com/repos/influxdb/influxdb/subscription","commits_url":"https://api.github.com/repos/influxdb/influxdb/commits{/sha}","git_commits_url":"https://api.github.com/repos/influxdb/influxdb/git/commits{/sha}","comments_url":"https://api.github.com/repos/influxdb/influxdb/comments{/number}","issue_comment_url":"https://api.github.com/repos/influxdb/influxdb/issues/comments{/number}","contents_url":"https://api.github.com/repos/influxdb/influxdb/contents/{+path}","compare_url":"https://api.github.com/repos/influxdb/influxdb/compare/{base}...{head}","merges_url":"https://api.github.com/repos/influxdb/influxdb/merges","archive_url":"https://api.github.com/repos/influxdb/influxdb/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/influxdb/influxdb/downloads","issues_url":"https://api.github.com/repos/influxdb/influxdb/issues{/number}","pulls_url":"https://api.github.com/repos/influxdb/influxdb/pulls{/number}","milestones_url":"https://api.github.com/repos/influxdb/influxdb/milestones{/number}","notifications_url":"https://api.github.com/repos/influxdb/influxdb/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/influxdb/influxdb/labels{/name}","releases_url":"https://api.github.com/repos/influxdb/influxdb/releases{/id}","created_at":"2013-09-26T14:31:10Z","updated_at":"2015-12-03T17:10:06Z","pushed_at":"2015-12-03T19:04:13Z","git_url":"git://github.com/influxdb/influxdb.git","ssh_url":"git@github.com:influxdb/influxdb.git","clone_url":"https://github.com/influxdb/influxdb.git","svn_url":"https://github.com/influxdb/influxdb","homepage":"http://influxdb.com","size":38767,"stargazers_count":6867,"watchers_count":6867,"language":"Go","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":831,"mirror_url":null,"open_issues_count":502,"forks":831,"open_issues":502,"watchers":6867,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/4966"},"html":{"href":"https://github.com/influxdb/influxdb/pull/4966"},"issue":{"href":"https://api.github.com/repos/influxdb/influxdb/issues/4966"},"comments":{"href":"https://api.github.com/repos/influxdb/influxdb/issues/4966/comments"},"review_comments":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/4966/comments"},"review_comment":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/influxdb/influxdb/pulls/4966/commits"},"statuses":{"href":"https://api.github.com/repos/influxdb/influxdb/statuses/4624fb2a78f51c0d55b2761d3070e15957ccc4ce"}}}}