{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/16575","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/16575/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/16575/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/16575/events","html_url":"https://github.com/cockroachdb/cockroach/pull/16575","id":236616995,"number":16575,"title":"storage: Balance based on ranges-per-GB of disk, not raw range count","user":{"login":"a-robinson","id":7085343,"avatar_url":"https://avatars0.githubusercontent.com/u/7085343?v=3","gravatar_id":"","url":"https://api.github.com/users/a-robinson","html_url":"https://github.com/a-robinson","followers_url":"https://api.github.com/users/a-robinson/followers","following_url":"https://api.github.com/users/a-robinson/following{/other_user}","gists_url":"https://api.github.com/users/a-robinson/gists{/gist_id}","starred_url":"https://api.github.com/users/a-robinson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/a-robinson/subscriptions","organizations_url":"https://api.github.com/users/a-robinson/orgs","repos_url":"https://api.github.com/users/a-robinson/repos","events_url":"https://api.github.com/users/a-robinson/events{/privacy}","received_events_url":"https://api.github.com/users/a-robinson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2017-06-16T22:54:36Z","updated_at":"2017-06-17T04:04:57Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/16575","html_url":"https://github.com/cockroachdb/cockroach/pull/16575","diff_url":"https://github.com/cockroachdb/cockroach/pull/16575.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/16575.patch"},"body":"This should work approximately the same as today for clusters where all\r\nnodes have the same disk size, but better in clusters where some nodes\r\nhave different size disks.\r\n\r\nAs proposed in https://github.com/cockroachdb/cockroach/pull/16296"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/309191697","html_url":"https://github.com/cockroachdb/cockroach/pull/16575#issuecomment-309191697","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/16575","id":309191697,"user":{"login":"BramGruneir","id":1614265,"avatar_url":"https://avatars2.githubusercontent.com/u/1614265?v=3","gravatar_id":"","url":"https://api.github.com/users/BramGruneir","html_url":"https://github.com/BramGruneir","followers_url":"https://api.github.com/users/BramGruneir/followers","following_url":"https://api.github.com/users/BramGruneir/following{/other_user}","gists_url":"https://api.github.com/users/BramGruneir/gists{/gist_id}","starred_url":"https://api.github.com/users/BramGruneir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/BramGruneir/subscriptions","organizations_url":"https://api.github.com/users/BramGruneir/orgs","repos_url":"https://api.github.com/users/BramGruneir/repos","events_url":"https://api.github.com/users/BramGruneir/events{/privacy}","received_events_url":"https://api.github.com/users/BramGruneir/received_events","type":"User","site_admin":false},"created_at":"2017-06-17T04:04:57Z","updated_at":"2017-06-17T04:04:57Z","body":"\n\n\n\nReviewed 3 of 3 files at r1.\nReview status: all files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.\n\n---\n\n*[pkg/storage/rule_solver.go, line 44 at r1](https://reviewable.io:443/reviews/cockroachdb/cockroach/16575#-KmoDIKbYB-IRaUCoy7-:-KmoDIKbYB-IRaUCoy70:bk8o53c) ([raw file](https://github.com/cockroachdb/cockroach/blob/3a7793e093dca65fd0e8d165ce5e3a012c30f91e/pkg/storage/rule_solver.go#L44)):*\n> ```Go\n> \n> \t// TODO: Is this a defined constant anywhere in a standard library?\n> \tbytesPerGB = 1 << 30\n> ```\n\nSuper nit: but shouldn't this be bytesPerGiB ... well... everywhere?\n\n---\n\n*[pkg/storage/rule_solver.go, line 60 at r1](https://reviewable.io:443/reviews/cockroachdb/cockroach/16575#-KmoCTAgMv5PIyFZ7NEb:-KmoCTAgMv5PIyFZ7NEc:b-wz0ml7) ([raw file](https://github.com/cockroachdb/cockroach/blob/3a7793e093dca65fd0e8d165ce5e3a012c30f91e/pkg/storage/rule_solver.go#L60)):*\n> ```Go\n> \tvalid           bool\n> \tconstraintScore float64\n> \trangesPerGB     float64\n> ```\n\nWhy make this a float and perform this bytesPerGB computation everywhere?  Why not just leave the value as bytes and only convert when formatting for human output (using the humanize library)?\n\n---\n\n*[pkg/storage/rule_solver.go, line 232 at r1](https://reviewable.io:443/reviews/cockroachdb/cockroach/16575#-KmoCz1WXte_XqBhAigE:-KmoCz1WXte_XqBhAigF:b-rquve7) ([raw file](https://github.com/cockroachdb/cockroach/blob/3a7793e093dca65fd0e8d165ce5e3a012c30f91e/pkg/storage/rule_solver.go#L232)):*\n> ```Go\n> \t// this is primarily useful to avoid huge rangesPerGB numbers for unit tests\n> \t// that use 1MB store capacities.\n> \tcapacityGB := math.Max(float64(capacity.Capacity/bytesPerGB), 1.0)\n> ```\n\nPerhaps keep the number of replicas around and fallback on count when less than 1 GiB?  \n\n---\n\n\n*Comments from [Reviewable](https://reviewable.io:443/reviews/cockroachdb/cockroach/16575)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"BramGruneir"}}