Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Rollup] Add new capabilities endpoint based on concrete rollup indices #30401

Merged
merged 14 commits into from
Jul 16, 2018

Conversation

polyfractal
Copy link
Contributor

@polyfractal polyfractal commented May 4, 2018

This adds a key query string parameter to the GetRollupCaps API, which allows the user to specify how they would like the capabilities grouped at the top level.

The default behavior is as before: grouping by index pattern. But if the user specifies key: rollup_index, it groups the capabilities by the concrete rollup index where the rollups are stored. The rest of the response is unchanged from before (shows a list of all the jobs in that index, their config, etc)

This introduces a new GetRollupIndexCaps API which allows the user to retrieve rollup capabilities of a specific rollup index (or index pattern). This is distinct from the existing RollupCaps endpoint; it's a subtle but distinct difference.

  • Multiple jobs can be stored in multiple indices and point to a single target data index pattern (logstash-*). The existing API finds capabilities/config of all jobs matching that data index pattern.
  • One rollup index can hold data from multiple jobs, targeting multiple data index patterns. This new API finds the capabilities based on the concrete rollup indices.

/cc @jen-huang

This adds a `key` parameter to the GetRollupCaps API, which allows the
user to specify how they would like the capabilities grouped at the
top level.

The default behavior is as before: grouping by index pattern.  But if
the user specifies `key: rollup_index`, it groups the capabilities
by the concrete rollup index where the rollups are stored.
@polyfractal polyfractal added >enhancement review :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data labels May 4, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@polyfractal
Copy link
Contributor Author

Jenkins, run gradle build tests

@polyfractal
Copy link
Contributor Author

/cc @colings86 since Jim is off on holiday for a while :)

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a comment about how the action should work.


public Request(String indexPattern) {
public Request(String indexPattern, String key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little tricky because if we are allow an index pattern to resolve to concrete indexes then we should allow IndicesOptions as well so the user can control how wildcard expansion etc. behaves. This also introduces a bit of weirdness because the GetRollupCapsAction is a cluster action (the name starts with cluster: which is correct for when you are specifying the index pattern option because you are effectively asking the job not the indexes but when you are asking for the indexes option the action should really be an indices action (the name should start with indices:). I wonder if this actually needs to be two different APIs? or whether there is a way to make this always an indices: action?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe two endpoints is best.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Durp, hit enter too soon.

Right now I don't think it's a problem, since the options are:

  1. "Index pattern" specified in the job, which we are not actually resolving as index patterns. These are just checking string equality against the job's defined index pattern.

  2. The new behavior introduced in this PR, which requests the rollup job's concrete index. This does not support patterns at the moment, only a single, explicit index name.

So the PR as it stands should be ok. But Kibana has said they would like to be able to specify patterns with the concrete rollup index (rollup_storage_*), which would circle back to the concerns you've raised.

@colings86
Copy link
Contributor

@polyfractal could you add the target versions to this PR please?

@polyfractal polyfractal changed the title [Rollup] Allow customization of how RollupCaps are grouped [Rollup] Add new capabilities endpoint based on concrete rollup indices Jun 26, 2018
@polyfractal
Copy link
Contributor Author

polyfractal commented Jun 26, 2018

Moved things over to a new endpoint, updated the OP description and changed the title. Still need to edit the docs.

Edit: lotsa test failures, i think i broke something :)

@polyfractal
Copy link
Contributor Author

Jenkins, run gradle build tests

@polyfractal
Copy link
Contributor Author

Tests now passing after finding that assertion issue, think this is ready for another review

Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@polyfractal polyfractal merged commit 791b9b1 into elastic:master Jul 16, 2018
polyfractal added a commit to polyfractal/elasticsearch that referenced this pull request Jul 17, 2018
…lastic#30401)

This introduces a new GetRollupIndexCaps API which allows the user to retrieve rollup capabilities of a specific rollup index (or index pattern). This is distinct from the existing RollupCaps endpoint.

- Multiple jobs can be stored in multiple indices and point to a single target data index pattern (logstash-*). The existing API finds capabilities/config of all jobs matching that data index pattern.
- One rollup index can hold data from multiple jobs, targeting multiple data index patterns. This new API finds the capabilities based on the concrete rollup indices.
martijnvg added a commit that referenced this pull request Jul 17, 2018
* es/master:
  Add Index UUID to `/_stats` Response (#31871)
  Painless: Move and Rename Several Methods in the lookup package (#32105)
  Bypass highlight query terms extraction on empty fields (#32090)
  Switch non-x-pack to new style requests (#32106)
  [Rollup] Add new capabilities endpoint for concrete rollup indices (#30401)
  Revert "[test] disable packaging tests for suse boxes"
  SQL: allow LEFT and RIGHT as function names (#32066)
  DOCS: put LIMIT 10 to the SQL query (#32065)
  [test] turn on host io cache for opensuse (#32053)
  Tweaked Elasticsearch Service links for SEO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants