{"action":"created","comment":{"url":"https://api.github.com/repos/atlassian/gostatsd/pulls/comments/133633286","pull_request_review_id":56836168,"id":133633286,"diff_hunk":"@@ -48,30 +52,61 @@ func NewMetricDispatcher(numWorkers int, perWorkerBufferSize int, af AggregatorF\n \t}\n }\n \n-// Run runs the MetricDispatcher.\n-func (d *MetricDispatcher) Run(ctx context.Context, done gostatsd.Done) {\n-\tdefer done()\n-\tvar wg sync.WaitGroup\n-\twg.Add(d.numWorkers)\n-\tfor _, worker := range d.workers {\n-\t\tw := worker // Make a copy of the loop variable! https://github.com/golang/go/wiki/CommonMistakes\n-\t\tgo w.work(wg.Done)\n-\t}\n-\tdefer func() {\n+// RunAsync runs the MetricDispatcher in a goroutine\n+func (d *MetricDispatcher) RunAsync(ctx context.Context, done gostatsd.Done) {\n+\td.lock.Lock()\n+\tdefer d.lock.Unlock()\n+\td.running = true\n+\tgo func() {\n+\t\tdefer done()\n+\n+\t\tvar wg sync.WaitGroup\n+\t\twg.Add(d.numWorkers)\n \t\tfor _, worker := range d.workers {\n-\t\t\tclose(worker.metricsQueue) // Close channel to terminate worker\n+\t\t\tw := worker // Make a copy of the loop variable! https://github.com/golang/go/wiki/CommonMistakes\n+\t\t\tgo w.work(wg.Done)\n \t\t}\n-\t\twg.Wait() // Wait for all workers to finish\n+\n+\t\tdefer func() {\n+\t\t\t// Once this is indicated as not running, DispatchMetric will no longer\n+\t\t\t// try to send to the channel, so it is safe to close.\n+\t\t\td.lock.Lock()\n+\t\t\td.running = false\n+\t\t\td.lock.Unlock()\n+\n+\t\t\tfor _, worker := range d.workers {\n+\t\t\t\tclose(worker.metricsQueue) // Close channel to terminate worker\n+\t\t\t}\n+\t\t\twg.Wait() // Wait for all workers to finish\n+\t\t}()\n+\n+\t\t// Work until asked to stop\n+\t\t<-ctx.Done()\n \t}()\n+}\n \n-\t// Work until asked to stop\n-\t<-ctx.Done()\n+func (d *MetricDispatcher) runMetrics(ctx context.Context, statser statser.Statser) {\n+\tfor _, worker := range d.workers {\n+\t\tworker.runMetrics(ctx, statser)\n+\t}\n+\td.Process(ctx, func(aggrId uint16, aggr Aggregator) {\n+\t\ttag := fmt.Sprintf(\"aggregator_id:%d\", aggrId)\n+\t\taggr.TrackMetrics(statser.WithTags(gostatsd.Tags{tag}))\n+\t})\n }\n \n // DispatchMetric dispatches metric to a corresponding Aggregator.\n func (d *MetricDispatcher) DispatchMetric(ctx context.Context, m *gostatsd.Metric) error {\n \thash := adler32.Checksum([]byte(m.Name))\n \tw := d.workers[uint16(hash%uint32(d.numWorkers))]\n+\n+\t// Protects both the read of d.running, and that the channel remains open\n+\td.lock.RLock()\n+\tdefer d.lock.RUnlock()\n+\tif !d.running {\n+\t\treturn nil\n+\t}\n+","path":"pkg/statsd/dispatcher.go","position":null,"original_position":108,"commit_id":"318e03300b2aad1d882c2a7035f8f2586a190db1","original_commit_id":"5d957669c531aac1d9073956283b5ade3692c254","user":{"login":"tiedotguy","id":29549188,"avatar_url":"https://avatars1.githubusercontent.com/u/29549188?v=4","gravatar_id":"","url":"https://api.github.com/users/tiedotguy","html_url":"https://github.com/tiedotguy","followers_url":"https://api.github.com/users/tiedotguy/followers","following_url":"https://api.github.com/users/tiedotguy/following{/other_user}","gists_url":"https://api.github.com/users/tiedotguy/gists{/gist_id}","starred_url":"https://api.github.com/users/tiedotguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiedotguy/subscriptions","organizations_url":"https://api.github.com/users/tiedotguy/orgs","repos_url":"https://api.github.com/users/tiedotguy/repos","events_url":"https://api.github.com/users/tiedotguy/events{/privacy}","received_events_url":"https://api.github.com/users/tiedotguy/received_events","type":"User","site_admin":false},"body":"Think I got it sorted now, see how this looks.","created_at":"2017-08-17T06:41:04Z","updated_at":"2017-08-17T06:41:04Z","html_url":"https://github.com/atlassian/gostatsd/pull/117#discussion_r133633286","pull_request_url":"https://api.github.com/repos/atlassian/gostatsd/pulls/117","_links":{"self":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/comments/133633286"},"html":{"href":"https://github.com/atlassian/gostatsd/pull/117#discussion_r133633286"},"pull_request":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/117"}}},"pull_request":{"url":"https://api.github.com/repos/atlassian/gostatsd/pulls/117","id":135943867,"html_url":"https://github.com/atlassian/gostatsd/pull/117","diff_url":"https://github.com/atlassian/gostatsd/pull/117.diff","patch_url":"https://github.com/atlassian/gostatsd/pull/117.patch","issue_url":"https://api.github.com/repos/atlassian/gostatsd/issues/117","number":117,"state":"open","locked":false,"title":"Refactor internal metrics to use a Statser","user":{"login":"tiedotguy","id":29549188,"avatar_url":"https://avatars1.githubusercontent.com/u/29549188?v=4","gravatar_id":"","url":"https://api.github.com/users/tiedotguy","html_url":"https://github.com/tiedotguy","followers_url":"https://api.github.com/users/tiedotguy/followers","following_url":"https://api.github.com/users/tiedotguy/following{/other_user}","gists_url":"https://api.github.com/users/tiedotguy/gists{/gist_id}","starred_url":"https://api.github.com/users/tiedotguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiedotguy/subscriptions","organizations_url":"https://api.github.com/users/tiedotguy/orgs","repos_url":"https://api.github.com/users/tiedotguy/repos","events_url":"https://api.github.com/users/tiedotguy/events{/privacy}","received_events_url":"https://api.github.com/users/tiedotguy/received_events","type":"User","site_admin":false},"body":"This touches on a whole heap of stuff, but the gist is that we currently have all these backdoory hooks, and adding more internal metrics means per-metric backdoory hooks.\r\n\r\nThis removes all of them, and replaces them with a Statser.  We still use use backdoory hooks to get the Statser where it needs to be, but then it's done.  It gives a better foundation to build internal metrics on.\r\n\r\nIt also gives us the facility to send to an alternate statsd service, which means we should retain metrics under failure-to-submit conditions.  Currently that's not implemented.  Configuring Statsers is also not implemented.\r\n\r\nMetrics now send on their own interval, rather than being slaved to the MetricFlusher, because they may not be going to the current flusher.  Intervals are also not yet configurable.\r\n\r\nIt also splits out an internal metric namespace and internal metric tags.  If we were any other service we would have this control over our internal metrics, so I want them for this service too.","created_at":"2017-08-16T05:33:53Z","updated_at":"2017-08-17T06:41:04Z","closed_at":null,"merged_at":null,"merge_commit_sha":"97ff662ac62c0bf19efe840cba557296012e3fa3","assignee":null,"assignees":[],"requested_reviewers":[{"login":"aelse","id":165789,"avatar_url":"https://avatars1.githubusercontent.com/u/165789?v=4","gravatar_id":"","url":"https://api.github.com/users/aelse","html_url":"https://github.com/aelse","followers_url":"https://api.github.com/users/aelse/followers","following_url":"https://api.github.com/users/aelse/following{/other_user}","gists_url":"https://api.github.com/users/aelse/gists{/gist_id}","starred_url":"https://api.github.com/users/aelse/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aelse/subscriptions","organizations_url":"https://api.github.com/users/aelse/orgs","repos_url":"https://api.github.com/users/aelse/repos","events_url":"https://api.github.com/users/aelse/events{/privacy}","received_events_url":"https://api.github.com/users/aelse/received_events","type":"User","site_admin":false}],"milestone":null,"commits_url":"https://api.github.com/repos/atlassian/gostatsd/pulls/117/commits","review_comments_url":"https://api.github.com/repos/atlassian/gostatsd/pulls/117/comments","review_comment_url":"https://api.github.com/repos/atlassian/gostatsd/pulls/comments{/number}","comments_url":"https://api.github.com/repos/atlassian/gostatsd/issues/117/comments","statuses_url":"https://api.github.com/repos/atlassian/gostatsd/statuses/318e03300b2aad1d882c2a7035f8f2586a190db1","head":{"label":"tiedotguy:refactor-internal-metrics","ref":"refactor-internal-metrics","sha":"318e03300b2aad1d882c2a7035f8f2586a190db1","user":{"login":"tiedotguy","id":29549188,"avatar_url":"https://avatars1.githubusercontent.com/u/29549188?v=4","gravatar_id":"","url":"https://api.github.com/users/tiedotguy","html_url":"https://github.com/tiedotguy","followers_url":"https://api.github.com/users/tiedotguy/followers","following_url":"https://api.github.com/users/tiedotguy/following{/other_user}","gists_url":"https://api.github.com/users/tiedotguy/gists{/gist_id}","starred_url":"https://api.github.com/users/tiedotguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiedotguy/subscriptions","organizations_url":"https://api.github.com/users/tiedotguy/orgs","repos_url":"https://api.github.com/users/tiedotguy/repos","events_url":"https://api.github.com/users/tiedotguy/events{/privacy}","received_events_url":"https://api.github.com/users/tiedotguy/received_events","type":"User","site_admin":false},"repo":{"id":94816630,"name":"gostatsd","full_name":"tiedotguy/gostatsd","owner":{"login":"tiedotguy","id":29549188,"avatar_url":"https://avatars1.githubusercontent.com/u/29549188?v=4","gravatar_id":"","url":"https://api.github.com/users/tiedotguy","html_url":"https://github.com/tiedotguy","followers_url":"https://api.github.com/users/tiedotguy/followers","following_url":"https://api.github.com/users/tiedotguy/following{/other_user}","gists_url":"https://api.github.com/users/tiedotguy/gists{/gist_id}","starred_url":"https://api.github.com/users/tiedotguy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tiedotguy/subscriptions","organizations_url":"https://api.github.com/users/tiedotguy/orgs","repos_url":"https://api.github.com/users/tiedotguy/repos","events_url":"https://api.github.com/users/tiedotguy/events{/privacy}","received_events_url":"https://api.github.com/users/tiedotguy/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/tiedotguy/gostatsd","description":"An implementation of Etsy's statsd in Go with tags support","fork":true,"url":"https://api.github.com/repos/tiedotguy/gostatsd","forks_url":"https://api.github.com/repos/tiedotguy/gostatsd/forks","keys_url":"https://api.github.com/repos/tiedotguy/gostatsd/keys{/key_id}","collaborators_url":"https://api.github.com/repos/tiedotguy/gostatsd/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/tiedotguy/gostatsd/teams","hooks_url":"https://api.github.com/repos/tiedotguy/gostatsd/hooks","issue_events_url":"https://api.github.com/repos/tiedotguy/gostatsd/issues/events{/number}","events_url":"https://api.github.com/repos/tiedotguy/gostatsd/events","assignees_url":"https://api.github.com/repos/tiedotguy/gostatsd/assignees{/user}","branches_url":"https://api.github.com/repos/tiedotguy/gostatsd/branches{/branch}","tags_url":"https://api.github.com/repos/tiedotguy/gostatsd/tags","blobs_url":"https://api.github.com/repos/tiedotguy/gostatsd/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/tiedotguy/gostatsd/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/tiedotguy/gostatsd/git/refs{/sha}","trees_url":"https://api.github.com/repos/tiedotguy/gostatsd/git/trees{/sha}","statuses_url":"https://api.github.com/repos/tiedotguy/gostatsd/statuses/{sha}","languages_url":"https://api.github.com/repos/tiedotguy/gostatsd/languages","stargazers_url":"https://api.github.com/repos/tiedotguy/gostatsd/stargazers","contributors_url":"https://api.github.com/repos/tiedotguy/gostatsd/contributors","subscribers_url":"https://api.github.com/repos/tiedotguy/gostatsd/subscribers","subscription_url":"https://api.github.com/repos/tiedotguy/gostatsd/subscription","commits_url":"https://api.github.com/repos/tiedotguy/gostatsd/commits{/sha}","git_commits_url":"https://api.github.com/repos/tiedotguy/gostatsd/git/commits{/sha}","comments_url":"https://api.github.com/repos/tiedotguy/gostatsd/comments{/number}","issue_comment_url":"https://api.github.com/repos/tiedotguy/gostatsd/issues/comments{/number}","contents_url":"https://api.github.com/repos/tiedotguy/gostatsd/contents/{+path}","compare_url":"https://api.github.com/repos/tiedotguy/gostatsd/compare/{base}...{head}","merges_url":"https://api.github.com/repos/tiedotguy/gostatsd/merges","archive_url":"https://api.github.com/repos/tiedotguy/gostatsd/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/tiedotguy/gostatsd/downloads","issues_url":"https://api.github.com/repos/tiedotguy/gostatsd/issues{/number}","pulls_url":"https://api.github.com/repos/tiedotguy/gostatsd/pulls{/number}","milestones_url":"https://api.github.com/repos/tiedotguy/gostatsd/milestones{/number}","notifications_url":"https://api.github.com/repos/tiedotguy/gostatsd/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/tiedotguy/gostatsd/labels{/name}","releases_url":"https://api.github.com/repos/tiedotguy/gostatsd/releases{/id}","deployments_url":"https://api.github.com/repos/tiedotguy/gostatsd/deployments","created_at":"2017-06-19T20:10:02Z","updated_at":"2017-06-19T20:10:04Z","pushed_at":"2017-08-17T06:39:49Z","git_url":"git://github.com/tiedotguy/gostatsd.git","ssh_url":"git@github.com:tiedotguy/gostatsd.git","clone_url":"https://github.com/tiedotguy/gostatsd.git","svn_url":"https://github.com/tiedotguy/gostatsd","homepage":"","size":609,"stargazers_count":0,"watchers_count":0,"language":"Go","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"atlassian:master","ref":"master","sha":"cfc76dd7d074884ac84466010d63316a2958f1c4","user":{"login":"atlassian","id":168166,"avatar_url":"https://avatars3.githubusercontent.com/u/168166?v=4","gravatar_id":"","url":"https://api.github.com/users/atlassian","html_url":"https://github.com/atlassian","followers_url":"https://api.github.com/users/atlassian/followers","following_url":"https://api.github.com/users/atlassian/following{/other_user}","gists_url":"https://api.github.com/users/atlassian/gists{/gist_id}","starred_url":"https://api.github.com/users/atlassian/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atlassian/subscriptions","organizations_url":"https://api.github.com/users/atlassian/orgs","repos_url":"https://api.github.com/users/atlassian/repos","events_url":"https://api.github.com/users/atlassian/events{/privacy}","received_events_url":"https://api.github.com/users/atlassian/received_events","type":"Organization","site_admin":false},"repo":{"id":55813517,"name":"gostatsd","full_name":"atlassian/gostatsd","owner":{"login":"atlassian","id":168166,"avatar_url":"https://avatars3.githubusercontent.com/u/168166?v=4","gravatar_id":"","url":"https://api.github.com/users/atlassian","html_url":"https://github.com/atlassian","followers_url":"https://api.github.com/users/atlassian/followers","following_url":"https://api.github.com/users/atlassian/following{/other_user}","gists_url":"https://api.github.com/users/atlassian/gists{/gist_id}","starred_url":"https://api.github.com/users/atlassian/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/atlassian/subscriptions","organizations_url":"https://api.github.com/users/atlassian/orgs","repos_url":"https://api.github.com/users/atlassian/repos","events_url":"https://api.github.com/users/atlassian/events{/privacy}","received_events_url":"https://api.github.com/users/atlassian/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/atlassian/gostatsd","description":"An implementation of Etsy's statsd in Go with tags support","fork":false,"url":"https://api.github.com/repos/atlassian/gostatsd","forks_url":"https://api.github.com/repos/atlassian/gostatsd/forks","keys_url":"https://api.github.com/repos/atlassian/gostatsd/keys{/key_id}","collaborators_url":"https://api.github.com/repos/atlassian/gostatsd/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/atlassian/gostatsd/teams","hooks_url":"https://api.github.com/repos/atlassian/gostatsd/hooks","issue_events_url":"https://api.github.com/repos/atlassian/gostatsd/issues/events{/number}","events_url":"https://api.github.com/repos/atlassian/gostatsd/events","assignees_url":"https://api.github.com/repos/atlassian/gostatsd/assignees{/user}","branches_url":"https://api.github.com/repos/atlassian/gostatsd/branches{/branch}","tags_url":"https://api.github.com/repos/atlassian/gostatsd/tags","blobs_url":"https://api.github.com/repos/atlassian/gostatsd/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/atlassian/gostatsd/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/atlassian/gostatsd/git/refs{/sha}","trees_url":"https://api.github.com/repos/atlassian/gostatsd/git/trees{/sha}","statuses_url":"https://api.github.com/repos/atlassian/gostatsd/statuses/{sha}","languages_url":"https://api.github.com/repos/atlassian/gostatsd/languages","stargazers_url":"https://api.github.com/repos/atlassian/gostatsd/stargazers","contributors_url":"https://api.github.com/repos/atlassian/gostatsd/contributors","subscribers_url":"https://api.github.com/repos/atlassian/gostatsd/subscribers","subscription_url":"https://api.github.com/repos/atlassian/gostatsd/subscription","commits_url":"https://api.github.com/repos/atlassian/gostatsd/commits{/sha}","git_commits_url":"https://api.github.com/repos/atlassian/gostatsd/git/commits{/sha}","comments_url":"https://api.github.com/repos/atlassian/gostatsd/comments{/number}","issue_comment_url":"https://api.github.com/repos/atlassian/gostatsd/issues/comments{/number}","contents_url":"https://api.github.com/repos/atlassian/gostatsd/contents/{+path}","compare_url":"https://api.github.com/repos/atlassian/gostatsd/compare/{base}...{head}","merges_url":"https://api.github.com/repos/atlassian/gostatsd/merges","archive_url":"https://api.github.com/repos/atlassian/gostatsd/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/atlassian/gostatsd/downloads","issues_url":"https://api.github.com/repos/atlassian/gostatsd/issues{/number}","pulls_url":"https://api.github.com/repos/atlassian/gostatsd/pulls{/number}","milestones_url":"https://api.github.com/repos/atlassian/gostatsd/milestones{/number}","notifications_url":"https://api.github.com/repos/atlassian/gostatsd/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/atlassian/gostatsd/labels{/name}","releases_url":"https://api.github.com/repos/atlassian/gostatsd/releases{/id}","deployments_url":"https://api.github.com/repos/atlassian/gostatsd/deployments","created_at":"2016-04-08T22:29:32Z","updated_at":"2017-08-16T16:29:37Z","pushed_at":"2017-08-17T06:39:50Z","git_url":"git://github.com/atlassian/gostatsd.git","ssh_url":"git@github.com:atlassian/gostatsd.git","clone_url":"https://github.com/atlassian/gostatsd.git","svn_url":"https://github.com/atlassian/gostatsd","homepage":"","size":588,"stargazers_count":101,"watchers_count":101,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":25,"mirror_url":null,"open_issues_count":15,"forks":25,"open_issues":15,"watchers":101,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/117"},"html":{"href":"https://github.com/atlassian/gostatsd/pull/117"},"issue":{"href":"https://api.github.com/repos/atlassian/gostatsd/issues/117"},"comments":{"href":"https://api.github.com/repos/atlassian/gostatsd/issues/117/comments"},"review_comments":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/117/comments"},"review_comment":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/atlassian/gostatsd/pulls/117/commits"},"statuses":{"href":"https://api.github.com/repos/atlassian/gostatsd/statuses/318e03300b2aad1d882c2a7035f8f2586a190db1"}}}}
{"actor":{"display_login":"tiedotguy"}}