Skip to content

Removing ml-state index from archive#77143

Merged
cauemarcondes merged 6 commits intoelastic:masterfrom
cauemarcondes:apm-api-test-ml-anomalies
Sep 10, 2020
Merged

Removing ml-state index from archive#77143
cauemarcondes merged 6 commits intoelastic:masterfrom
cauemarcondes:apm-api-test-ml-anomalies

Conversation

@cauemarcondes
Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes commented Sep 10, 2020

This backport PR #76854 is failing because of ES incompatibility versions on ml-state index.

info [r.suppressed] [Caues-MacBook-Pro.local] path: /.ml-state-000001, params: {include_type_name=true, index=.ml-state-000001}
   │      java.lang.ClassCastException: class java.lang.Boolean cannot be cast to class java.util.Map (java.lang.Boolean and java.util.Map are in module java.base of loader 'bootstrap')
   │      	at org.elasticsearch.action.admin.indices.create.CreateIndexRequest.source(CreateIndexRequest.java:406) ~[elasticsearch-7.10.0-SNAPSHOT.jar:7.10.0-SNAPSHOT]

Since this is not an important index to test ML anomalies I created a new archive without it.

@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 10, 2020
@dgieselaar
Copy link
Copy Markdown
Contributor

dgieselaar commented Sep 10, 2020

@cauemarcondes btw, why do we have a separate archive for anomalies? the main archive (apm_8.0.0) has anomalies. Can we just replace ml_8.0.0 with that archive?

@cauemarcondes
Copy link
Copy Markdown
Contributor Author

cauemarcondes commented Sep 10, 2020

@cauemarcondes btw, why do we have a separate archive for anomalies? the main archive (apm_8.0.0) has anomalies. Can we just replace ml_8.0.0 with that archive?

@dgieselaar I added it when we talked last week that you needed to have data with anomalies. #75528 (comment). Well now that you've added a new archive that contains anomalies I suppose I can delete the ml_8.0.0. Are you ok?

@dgieselaar
Copy link
Copy Markdown
Contributor

dgieselaar commented Sep 10, 2020

@cauemarcondes yes, let's delete this one!

dgieselaar
dgieselaar reviewed Sep 10, 2020
View reviewed changes
x-pack/test/apm_api_integration/trial/tests/service_maps/service_maps.ts
});
});

describe('when there is data with anomalies', () => {
Copy link
Copy Markdown
Contributor

@dgieselaar dgieselaar Sep 10, 2020

Choose a reason for hiding this comment

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

Why is this test removed?

Copy link
Copy Markdown
Contributor Author

@cauemarcondes cauemarcondes Sep 10, 2020

Choose a reason for hiding this comment

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

I want to make my backport PR to pass, and it doesn't have any relation to the issue I was solving there. I will open a new PR adding it back.

Copy link
Copy Markdown
Contributor

@dgieselaar dgieselaar Sep 10, 2020

Choose a reason for hiding this comment

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

Can you clarify? Not sure why we need to remove this. Do you think the new archive breaks on ES 7.9.0?

Copy link
Copy Markdown
Contributor Author

@cauemarcondes cauemarcondes Sep 10, 2020

Choose a reason for hiding this comment

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

I added this test on my PR that fixed the error rate, https://github.com/elastic/kibana/pull/75528/files#diff-efea71f9ccdbac05c7ca5d5ac4a993f3R289, I did to make sure that the new ML_8.0.0 archiver had anomalies. But now that I am going to delete this archive, I'm going to delete also the test, so I can backport the error rate fix to 7.x.

Copy link
Copy Markdown
Contributor

@dgieselaar dgieselaar Sep 10, 2020

Choose a reason for hiding this comment

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

But, this is a test that checks the service maps for anomaly data. We need it for 7.9 as well right? FWIW, I don't expect the backport from this PR to cause issues, because .ml-state should no longer be part of the archive. You should be able to use apm_8.0.0 instead of ml_8.0.0. Am I missing something?

Copy link
Copy Markdown
Contributor Author

@cauemarcondes cauemarcondes Sep 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this is a test that checks the service maps for anomaly data.

Yeah, that's correct, but anyway I'll add the test back, change the archive, and fix it.

dgieselaar
dgieselaar approved these changes Sep 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@dgieselaar dgieselaar left a comment

Choose a reason for hiding this comment

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

Feels like we can improve the test a bit. Other than that, LGTM.

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Sep 10, 2020

💚 Build Succeeded

Build metrics

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cauemarcondes cauemarcondes merged commit e3c71a7 into elastic:master Sep 10, 2020
@cauemarcondes cauemarcondes deleted the apm-api-test-ml-anomalies branch September 10, 2020 20:42
@cauemarcondes cauemarcondes mentioned this pull request Sep 11, 2020
Closed
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Sep 11, 2020
@cauemarcondes
Removing ml-state index from archive (elastic#77143)
7721277 
* removing ml-state index from archive

* removing archiver

* removing archiver

* adding archiver with ml anomalies

* fixing test
# Conflicts:
#	x-pack/test/apm_api_integration/common/fixtures/es_archiver/apm_8.0.0/data.json.gz
#	x-pack/test/apm_api_integration/trial/archives_metadata.ts
#	x-pack/test/apm_api_integration/trial/fixtures/es_archiver/ml_8.0.0/data.json.gz
#	x-pack/test/apm_api_integration/trial/fixtures/es_archiver/ml_8.0.0/mappings.json
@cauemarcondes cauemarcondes mentioned this pull request Sep 13, 2020
Merged
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Sep 13, 2020
@cauemarcondes
Removing ml-state index from archive (elastic#77143)
06ca525 
* removing ml-state index from archive

* removing archiver

* removing archiver

* adding archiver with ml anomalies

* fixing test
# Conflicts:
#	x-pack/test/apm_api_integration/trial/tests/service_maps/service_maps.ts
cauemarcondes added a commit that referenced this pull request Sep 13, 2020
@cauemarcondes
Removing ml-state index from archive (#77143) (#77292)
e96eff6 
* removing ml-state index from archive

* removing archiver

* removing archiver

* adding archiver with ml anomalies

* fixing test
# Conflicts:
#	x-pack/test/apm_api_integration/trial/tests/service_maps/service_maps.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 14, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/data-tier…
50d99be 
…s-for-710

* 'master' of github.com:elastic/kibana: (65 commits)
  Separate url forwarding logic and legacy services (elastic#76892)
  Bump yargs-parser to v13.1.2+ (elastic#77009)
  [Ingest Manager] Shared Fleet agent policy action (elastic#76013)
  [Search] Re-add support for aborting when a connection is closed (elastic#76470)
  [Search] Remove long-running query pop-up (elastic#75385)
  [Monitoring] Fix UI error when alerting is not available (elastic#77179)
  do not log plugin id format warning in dist mode (elastic#77134)
  [ML] Improving client side error handling (elastic#76743)
  [Alerting][Connectors] Refactor IBM Resilient: Generic Implementation (phase one) (elastic#74357)
  [Docs] some basic searchsource api docs (elastic#77038)
  add  cGroupOverrides to the legacy config (elastic#77180)
  Change saved object bulkUpdate to work across multiple namespaces (elastic#75478)
  [Security Solution][Resolver] Replace Selectable popover with badges (elastic#76997)
  Removing ml-state index from archive (elastic#77143)
  [Security Solution] Add unit tests for histograms (elastic#77081)
  [Lens] Filters aggregation  (elastic#75635)
  [Enterprise Search] Update WS Overview logic to use new config data (elastic#77122)
  Cleanup type output before building new types (elastic#77211)
  [Security Solution] Use safe type in resolver backend (elastic#76969)
  Use proper lodash syntax (elastic#77105)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/node_allocation.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@dgieselaar dgieselaar dgieselaar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.10.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cauemarcondes @dgieselaar @kibanamachine