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.

Sign up for GitHub

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

Jump to bottom

Removing ml-state index from archive #77143

Merged
merged 6 commits into from
Sep 10, 2020
Merged

Removing ml-state index from archive #77143

merged 6 commits into from
Sep 10, 2020

Conversation

cauemarcondes
Copy link
Contributor

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
Member

@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
Contributor Author

@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
Member

@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
@@ -285,37 +285,5 @@ export default function serviceMapsApiTests({ getService }: FtrProviderContext)
});
});
});

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

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
Contributor Author

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
Member

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
Contributor Author

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
Member

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
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
Member

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

x-pack/test/apm_api_integration/trial/tests/service_maps/service_maps.ts Show resolved Hide resolved
@kibanamachine
Copy link
Contributor

💚 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

@dgieselaar dgieselaar dgieselaar approved these changes

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