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

[7.x] Fix an NPE in TransportPutRollupJobAction #74843

Merged
merged 1 commit into from
Jul 1, 2021

Conversation

joegallo
Copy link
Contributor

@joegallo joegallo commented Jul 1, 2021

Note: This issue only affects 7.x -- master already works as intended.

We return an error either way, but this way we're returning the intended "you can't do that and here's why" error rather than something the user wouldn't grok.

Kinda related to #32965, but I think this might go all the way back to when rollups were first introduced.

Reproduction

PUT foo
{
  "mappings": {
    "properties": {
      "the_field": { "type": "date" },
     "value_field": { "type": "integer" }
    }
  }
}

PUT _rollup/job/foo-1
{
  "index_pattern": "foo",
  "rollup_index": "non-existent-index",
  "cron": "*/30 * * * * ?",
  "page_size" :10,
  "groups" : {
    "date_histogram": {
      "field": "the_field",
      "calendar_interval": "1h"
    }
  },
  "metrics": [
    {
      "field": "value_field",
      "metrics": ["min", "max", "sum"]
    }
  ]
}

PUT non-rollup

PUT _rollup/job/foo-2
{
  "index_pattern": "foo",
  "rollup_index": "non-rollup",
  "cron": "*/30 * * * * ?",
  "page_size" :10,
  "groups" : {
    "date_histogram": {
      "field": "the_field",
      "calendar_interval": "1h"
    }
  },
  "metrics": [
    {
      "field": "value_field",
      "metrics": ["min", "max", "sum"]
    }
  ]
}

7.x error without this change

{
  "error" : {
  "root_cause" : [
      {
        "type" : "runtime_exception",
        "reason" : "Could not update mappings for rollup job [foo-2]"
      }
    ],
    "type" : "runtime_exception",
    "reason" : "Could not update mappings for rollup job [foo-2]",
    "caused_by" : {
      "type" : "null_pointer_exception",
      "reason" : "Cannot invoke \"org.elasticsearch.cluster.metadata.MappingMetadata.getSourceAsMap()\" because \"mappings\" is null"
    }
  },
  "status" : 500
}

master error without this change, 7.x error with this change

{
  "error" : {
    "root_cause" : [
      {
        "type" : "runtime_exception",
        "reason" : "Rollup data cannot be added to existing indices that contain non-rollup data (expected to find _meta key in mapping of rollup index [non-rollup] but not found)."
      }
    ],
    "type" : "runtime_exception",
    "reason" : "Rollup data cannot be added to existing indices that contain non-rollup data (expected to find _meta key in mapping of rollup index [non-rollup] but not found)."
  },
  "status" : 500
}

We return an error either way, but this way we're returning the "you
can't do that and here's why" error rather than something the user
wouldn't grok.
@joegallo joegallo added >bug :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data labels Jul 1, 2021
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jul 1, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@joegallo joegallo requested a review from csoulios July 1, 2021 14:45
@joegallo joegallo changed the title Fix an NPE in TransportPutRollupJobAction [7.x] Fix an NPE in TransportPutRollupJobAction Jul 1, 2021
Copy link
Contributor

@csoulios csoulios left a comment

Choose a reason for hiding this comment

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

LGTM!

@joegallo joegallo merged commit 175cd6c into elastic:7.x Jul 1, 2021
@joegallo joegallo deleted the transport-put-rollup-npe-7.x branch July 1, 2021 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug >non-issue :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v7.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants