{"action":"created","issue":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/32651","repository_url":"https://api.github.com/repos/kubernetes/kubernetes","labels_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/32651/labels{/name}","comments_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/32651/comments","events_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/32651/events","html_url":"https://github.com/kubernetes/kubernetes/pull/32651","id":176842416,"number":32651,"title":"Scheduler map reduce 3","user":{"login":"wojtek-t","id":10743879,"avatar_url":"https://avatars.githubusercontent.com/u/10743879?v=3","gravatar_id":"","url":"https://api.github.com/users/wojtek-t","html_url":"https://github.com/wojtek-t","followers_url":"https://api.github.com/users/wojtek-t/followers","following_url":"https://api.github.com/users/wojtek-t/following{/other_user}","gists_url":"https://api.github.com/users/wojtek-t/gists{/gist_id}","starred_url":"https://api.github.com/users/wojtek-t/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wojtek-t/subscriptions","organizations_url":"https://api.github.com/users/wojtek-t/orgs","repos_url":"https://api.github.com/users/wojtek-t/repos","events_url":"https://api.github.com/users/wojtek-t/events{/privacy}","received_events_url":"https://api.github.com/users/wojtek-t/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/cla:%20yes","name":"cla: yes","color":"bfe5bf"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/release-note-none","name":"release-note-none","color":"c2e0c6"},{"url":"https://api.github.com/repos/kubernetes/kubernetes/labels/size/L","name":"size/L","color":"ee9900"}],"state":"open","locked":false,"assignee":{"login":"gmarek","id":1703842,"avatar_url":"https://avatars.githubusercontent.com/u/1703842?v=3","gravatar_id":"","url":"https://api.github.com/users/gmarek","html_url":"https://github.com/gmarek","followers_url":"https://api.github.com/users/gmarek/followers","following_url":"https://api.github.com/users/gmarek/following{/other_user}","gists_url":"https://api.github.com/users/gmarek/gists{/gist_id}","starred_url":"https://api.github.com/users/gmarek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gmarek/subscriptions","organizations_url":"https://api.github.com/users/gmarek/orgs","repos_url":"https://api.github.com/users/gmarek/repos","events_url":"https://api.github.com/users/gmarek/events{/privacy}","received_events_url":"https://api.github.com/users/gmarek/received_events","type":"User","site_admin":false},"assignees":[{"login":"gmarek","id":1703842,"avatar_url":"https://avatars.githubusercontent.com/u/1703842?v=3","gravatar_id":"","url":"https://api.github.com/users/gmarek","html_url":"https://github.com/gmarek","followers_url":"https://api.github.com/users/gmarek/followers","following_url":"https://api.github.com/users/gmarek/following{/other_user}","gists_url":"https://api.github.com/users/gmarek/gists{/gist_id}","starred_url":"https://api.github.com/users/gmarek/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gmarek/subscriptions","organizations_url":"https://api.github.com/users/gmarek/orgs","repos_url":"https://api.github.com/users/gmarek/repos","events_url":"https://api.github.com/users/gmarek/events{/privacy}","received_events_url":"https://api.github.com/users/gmarek/received_events","type":"User","site_admin":false}],"milestone":null,"comments":5,"created_at":"2016-09-14T08:11:32Z","updated_at":"2016-09-27T09:27:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/kubernetes/kubernetes/pulls/32651","html_url":"https://github.com/kubernetes/kubernetes/pull/32651","diff_url":"https://github.com/kubernetes/kubernetes/pull/32651.diff","patch_url":"https://github.com/kubernetes/kubernetes/pull/32651.patch"},"body":"Ref https://github.com/kubernetes/kubernetes/issues/24246\n\n<!-- Reviewable:start -->\n---\nThis change is [<img src=\"https://reviewable.kubernetes.io/review_button.svg\" height=\"34\" align=\"absmiddle\" alt=\"Reviewable\"/>](https://reviewable.kubernetes.io/reviews/kubernetes/kubernetes/32651)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/kubernetes/kubernetes/issues/comments/249813714","html_url":"https://github.com/kubernetes/kubernetes/pull/32651#issuecomment-249813714","issue_url":"https://api.github.com/repos/kubernetes/kubernetes/issues/32651","id":249813714,"user":{"login":"wojtek-t","id":10743879,"avatar_url":"https://avatars.githubusercontent.com/u/10743879?v=3","gravatar_id":"","url":"https://api.github.com/users/wojtek-t","html_url":"https://github.com/wojtek-t","followers_url":"https://api.github.com/users/wojtek-t/followers","following_url":"https://api.github.com/users/wojtek-t/following{/other_user}","gists_url":"https://api.github.com/users/wojtek-t/gists{/gist_id}","starred_url":"https://api.github.com/users/wojtek-t/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wojtek-t/subscriptions","organizations_url":"https://api.github.com/users/wojtek-t/orgs","repos_url":"https://api.github.com/users/wojtek-t/repos","events_url":"https://api.github.com/users/wojtek-t/events{/privacy}","received_events_url":"https://api.github.com/users/wojtek-t/received_events","type":"User","site_admin":false},"created_at":"2016-09-27T09:27:48Z","updated_at":"2016-09-27T09:27:48Z","body":"\n\n\n\nReview status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.\n\n---\n\n*[plugin/pkg/scheduler/algorithm/types.go, line 36 at r1](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651#-KSertyduehLbkPgobZP:-KSexxNcHQEUD0F5GB6u:b-pns57g) ([raw file](https://github.com/kubernetes/kubernetes/blob/832f4ebc5cf602652e61dd3419c973241a19a3af/plugin/pkg/scheduler/algorithm/types.go#L36)):*\n<details><summary><i>Previously, gmarek (Marek Grabowski) wrote…</i></summary>\n> See my later comment - I don't think it's needed.\n</details>\nSee my later comments - I think it currently is needed.\n\n---\n\n*[plugin/pkg/scheduler/algorithm/priorities/node_affinity.go, line 34 at r1](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651#-KSeoTwuJjSHnLZjteU4:-KSexkMnV5Si43TaJxXg:bj6hmhy) ([raw file](https://github.com/kubernetes/kubernetes/blob/832f4ebc5cf602652e61dd3419c973241a19a3af/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go#L34)):*\n<details><summary><i>Previously, gmarek (Marek Grabowski) wrote…</i></summary>\n> I still don't like the `interface{}` here... Why not *PriorityMetadata? I see only one call to the `Map` and we know the type of meta there. You can always pass `nil` to indicate that this function needs to recompute affinities.\n</details>\nDiscussed offline. I wouldn't like to change this in this PR. I'm adding a TODO to change it after all related PRs are done (this change should be pretty simple).\n\n---\n\n*[plugin/pkg/scheduler/algorithm/priorities/node_affinity.go, line 64 at r1](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651#-KSeps6PLSb2kr37SC-5:-KSexKR-yXKcSIKCI24P:b-f35ws1) ([raw file](https://github.com/kubernetes/kubernetes/blob/832f4ebc5cf602652e61dd3419c973241a19a3af/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go#L64)):*\n<details><summary><i>Previously, gmarek (Marek Grabowski) wrote…</i></summary>\n> A thought - I guess in most of the cases there won't be many \"kinds\" of Nodes in the cluster, where by \"kind\" I understand the label set. We probably can do label-based-per-node computations on abstraction classes. (Not related to this PR)\n</details>\nYeah - there are still a bunch of things in scheduler that we can improve. This is one of those.\n\n---\n\n*[plugin/pkg/scheduler/algorithm/priorities/node_affinity.go, line 97 at r1](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651#-KSeqzPTwY60nrKD7fqM:-KSexRKMH3_gLZITn0Ct:bkbb2jd) ([raw file](https://github.com/kubernetes/kubernetes/blob/832f4ebc5cf602652e61dd3419c973241a19a3af/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go#L97)):*\n<details><summary><i>Previously, gmarek (Marek Grabowski) wrote…</i></summary>\n> This basically prints scores for all Nodes, so it can be moved to the Map function. Then Reduce won't need to get `*api.Pod`.\n</details>\nNot really - in Map you don't have full result - after Map phase you have just some numbers, that are then translated to result.\n\n---\n\n*[plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go, line 108 at r1](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651#-KSes6QxZ8uHV_PrahA0:-KSexhN7T9m7Mo4T4jlq:bkbb2jd) ([raw file](https://github.com/kubernetes/kubernetes/blob/832f4ebc5cf602652e61dd3419c973241a19a3af/plugin/pkg/scheduler/algorithm/priorities/taint_toleration.go#L108)):*\n<details><summary><i>Previously, gmarek (Marek Grabowski) wrote…</i></summary>\n> Again - this can be moved to Map.\n</details>\nNot really - in Map you don't have full result - after Map phase you have just some numbers, that are then translated to result.\n\n---\n\n\n*Comments from [Reviewable](https://reviewable.kubernetes.io:443/reviews/kubernetes/kubernetes/32651)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"wojtek-t"}}