{"action":"created","issue":{"url":"https://api.github.com/repos/elastic/elasticsearch/issues/30467","repository_url":"https://api.github.com/repos/elastic/elasticsearch","labels_url":"https://api.github.com/repos/elastic/elasticsearch/issues/30467/labels{/name}","comments_url":"https://api.github.com/repos/elastic/elasticsearch/issues/30467/comments","events_url":"https://api.github.com/repos/elastic/elasticsearch/issues/30467/events","html_url":"https://github.com/elastic/elasticsearch/issues/30467","id":321335464,"node_id":"MDU6SXNzdWUzMjEzMzU0NjQ=","number":30467,"title":"[Rollup] Ordering by sub-agg fails due to name rewriting","user":{"login":"polyfractal","id":1224228,"node_id":"MDQ6VXNlcjEyMjQyMjg=","avatar_url":"https://avatars1.githubusercontent.com/u/1224228?v=4","gravatar_id":"","url":"https://api.github.com/users/polyfractal","html_url":"https://github.com/polyfractal","followers_url":"https://api.github.com/users/polyfractal/followers","following_url":"https://api.github.com/users/polyfractal/following{/other_user}","gists_url":"https://api.github.com/users/polyfractal/gists{/gist_id}","starred_url":"https://api.github.com/users/polyfractal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/polyfractal/subscriptions","organizations_url":"https://api.github.com/users/polyfractal/orgs","repos_url":"https://api.github.com/users/polyfractal/repos","events_url":"https://api.github.com/users/polyfractal/events{/privacy}","received_events_url":"https://api.github.com/users/polyfractal/received_events","type":"User","site_admin":false},"labels":[{"id":912843355,"node_id":"MDU6TGFiZWw5MTI4NDMzNTU=","url":"https://api.github.com/repos/elastic/elasticsearch/labels/:Search/Rollup","name":":Search/Rollup","color":"0e8a16","default":false},{"id":23173,"node_id":"MDU6TGFiZWwyMzE3Mw==","url":"https://api.github.com/repos/elastic/elasticsearch/labels/%3Ebug","name":">bug","color":"b60205","default":false}],"state":"open","locked":false,"assignee":{"login":"polyfractal","id":1224228,"node_id":"MDQ6VXNlcjEyMjQyMjg=","avatar_url":"https://avatars1.githubusercontent.com/u/1224228?v=4","gravatar_id":"","url":"https://api.github.com/users/polyfractal","html_url":"https://github.com/polyfractal","followers_url":"https://api.github.com/users/polyfractal/followers","following_url":"https://api.github.com/users/polyfractal/following{/other_user}","gists_url":"https://api.github.com/users/polyfractal/gists{/gist_id}","starred_url":"https://api.github.com/users/polyfractal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/polyfractal/subscriptions","organizations_url":"https://api.github.com/users/polyfractal/orgs","repos_url":"https://api.github.com/users/polyfractal/repos","events_url":"https://api.github.com/users/polyfractal/events{/privacy}","received_events_url":"https://api.github.com/users/polyfractal/received_events","type":"User","site_admin":false},"assignees":[{"login":"polyfractal","id":1224228,"node_id":"MDQ6VXNlcjEyMjQyMjg=","avatar_url":"https://avatars1.githubusercontent.com/u/1224228?v=4","gravatar_id":"","url":"https://api.github.com/users/polyfractal","html_url":"https://github.com/polyfractal","followers_url":"https://api.github.com/users/polyfractal/followers","following_url":"https://api.github.com/users/polyfractal/following{/other_user}","gists_url":"https://api.github.com/users/polyfractal/gists{/gist_id}","starred_url":"https://api.github.com/users/polyfractal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/polyfractal/subscriptions","organizations_url":"https://api.github.com/users/polyfractal/orgs","repos_url":"https://api.github.com/users/polyfractal/repos","events_url":"https://api.github.com/users/polyfractal/events{/privacy}","received_events_url":"https://api.github.com/users/polyfractal/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2018-05-08T20:28:53Z","updated_at":"2018-08-22T17:05:05Z","closed_at":null,"author_association":"MEMBER","body":"If an aggregation tries to order by a sub-aggregation, RollupSearch will fail because we rewrite the aggregation name internally (`foo` might be rewritten to `foo.max.value`).  For example:\r\n\r\n```\r\nGET metricbeat_rollup/_rollup_search\r\n{\r\n  \"size\": 0,\r\n  \"aggs\": {\r\n    \"2\": {\r\n      \"date_histogram\": {\r\n        \"field\": \"@timestamp\",\r\n        \"interval\": \"1d\"\r\n      },\r\n      \"aggs\": {\r\n        \"3\": {\r\n          \"terms\": {\r\n            \"field\": \"host\",\r\n            \"size\": 5,\r\n            \"order\": {\r\n              \"1\": \"desc\"\r\n            }\r\n          },\r\n          \"aggs\": {\r\n            \"1\": {\r\n              \"avg\": {\r\n                \"field\": \"system.memory.actual.used.pct\"\r\n              }\r\n            }\r\n          }\r\n        }\r\n      }\r\n    }\r\n  }\r\n}\r\n```\r\n\r\nWill throw:\r\n\r\n```\r\n{\r\n  \"error\": {\r\n    \"root_cause\": [\r\n      {\r\n        \"type\": \"runtime_exception\",\r\n        \"reason\": \"all shards failed\"\r\n      }\r\n    ],\r\n    \"type\": \"runtime_exception\",\r\n    \"reason\": \"all shards failed\",\r\n    \"caused_by\": {\r\n      \"type\": \"search_phase_execution_exception\",\r\n      \"reason\": \"all shards failed\",\r\n      \"phase\": \"query\",\r\n      \"grouped\": true,\r\n      \"failed_shards\": [\r\n        {\r\n          \"shard\": 0,\r\n          \"index\": \"metricbeat_rollup\",\r\n          \"node\": \"5cEuSOkeSKeI8R4oCTvPug\",\r\n          \"reason\": {\r\n            \"type\": \"aggregation_execution_exception\",\r\n            \"reason\": \"Invalid aggregator order path [1]. Unknown aggregation [1]\"\r\n          }\r\n        }\r\n      ]\r\n    }\r\n  },\r\n  \"status\": 500\r\n}\r\n```\r\n\r\nThis is trivially fixable for most of the metrics, we just need to do the rewriting.  I'm not quite sure how this will work for averages though, since an avg == two metrics which we re-combine. We can't order by either agg individually since it only contains half the information, and if we order after the recombination we may be missing the \"best\" buckets.  \r\n\r\nThe only saving grace is that ordering by sub-agg is discouraged anyway as count errors are unbounded, so this isn't really any worse.  Ordering by sub-agg is as good as rolling dice :) \r\n\r\nI've been wanting to look into modifying `avg` to accept \"arbitrary\" counts from the doc (instead of just incrementing the counter), so perhaps this is another motivation to do so.\r\n\r\n/cc @cdahlqvist "},"comment":{"url":"https://api.github.com/repos/elastic/elasticsearch/issues/comments/415105394","html_url":"https://github.com/elastic/elasticsearch/issues/30467#issuecomment-415105394","issue_url":"https://api.github.com/repos/elastic/elasticsearch/issues/30467","id":415105394,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNTEwNTM5NA==","user":{"login":"polyfractal","id":1224228,"node_id":"MDQ6VXNlcjEyMjQyMjg=","avatar_url":"https://avatars1.githubusercontent.com/u/1224228?v=4","gravatar_id":"","url":"https://api.github.com/users/polyfractal","html_url":"https://github.com/polyfractal","followers_url":"https://api.github.com/users/polyfractal/followers","following_url":"https://api.github.com/users/polyfractal/following{/other_user}","gists_url":"https://api.github.com/users/polyfractal/gists{/gist_id}","starred_url":"https://api.github.com/users/polyfractal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/polyfractal/subscriptions","organizations_url":"https://api.github.com/users/polyfractal/orgs","repos_url":"https://api.github.com/users/polyfractal/repos","events_url":"https://api.github.com/users/polyfractal/events{/privacy}","received_events_url":"https://api.github.com/users/polyfractal/received_events","type":"User","site_admin":false},"created_at":"2018-08-22T17:05:05Z","updated_at":"2018-08-22T17:05:05Z","author_association":"MEMBER","body":"@zhuhuizj I looked into this the other day, and I'm afraid there's no quick fix for sorting the `avg` aggregation.  \r\n\r\nWhat we need is an average aggregator that can extract the count from the document, instead of just incrementing the count for each value.  That way we could perform the avg in a single agg instead of two like today, and it'd also allow us to sort.\r\n\r\nA while back, I merged a new `weighted_avg` aggregator (https://github.com/elastic/elasticsearch/pull/31037), which did some infrastructure work that would allow such an aggregation.  Now we need to implement that \"special\" average aggregation.  We could extend the existing `avg`, or make an entirely new one that lives just in Rollup.\r\n\r\nIn either case, we have a tentative plan but need to find time to work on it."}}
{"actor":{"display_login":"polyfractal"}}