[Rollup] Ordering by sub-agg fails due to name rewriting

[polyfractal]

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:

GET metricbeat_rollup/_rollup_search
{
  "size": 0,
  "aggs": {
    "2": {
      "date_histogram": {
        "field": "@timestamp",
        "interval": "1d"
      },
      "aggs": {
        "3": {
          "terms": {
            "field": "host",
            "size": 5,
            "order": {
              "1": "desc"
            }
          },
          "aggs": {
            "1": {
              "avg": {
                "field": "system.memory.actual.used.pct"
              }
            }
          }
        }
      }
    }
  }
}

Will throw:

{
  "error": {
    "root_cause": [
      {
        "type": "runtime_exception",
        "reason": "all shards failed"
      }
    ],
    "type": "runtime_exception",
    "reason": "all shards failed",
    "caused_by": {
      "type": "search_phase_execution_exception",
      "reason": "all shards failed",
      "phase": "query",
      "grouped": true,
      "failed_shards": [
        {
          "shard": 0,
          "index": "metricbeat_rollup",
          "node": "5cEuSOkeSKeI8R4oCTvPug",
          "reason": {
            "type": "aggregation_execution_exception",
            "reason": "Invalid aggregator order path [1]. Unknown aggregation [1]"
          }
        }
      ]
    }
  },
  "status": 500
}

This 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.

The 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 :)

I'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.

/cc @cdahlqvist

[elasticmachine]

Pinging @elastic/es-search-aggs

[zhuhuizj]

hi @polyfractal ，we're testing the rollup feature on es 6.3.2. I found max/min sub-aggs can be used in orderby, but avg still not work. Do we plan to fix this?

[polyfractal]

@zhuhuizj I looked into this the other day, and I'm afraid there's no quick fix for sorting the avg aggregation.

What 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.

A while back, I merged a new weighted_avg aggregator (#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.

In either case, we have a tentative plan but need to find time to work on it.

[zhuhuizj]

I see. thanks for your response:)

[polyfractal]

No problem! We'll ping this thread if we have any progress or updates to share, so feel free to subscribe to it for updates.

[polyfractal]

Closing as this is unlikely to be resolved with v1, and will be a non-issue with rollup v2