Skip to content

Conversation

@davidkyle
Copy link
Member

Adds a secondary sort to ML get scheduled events. Fixes a rare test failure in the yml tests where the order of returned events is not deterministic.

@davidkyle davidkyle added >non-issue >test Issues or PRs that are addressing/adding tests v7.0.0 v6.3.0 :ml Machine learning v6.4.0 labels Jun 4, 2018
@davidkyle davidkyle requested a review from droberts195 June 4, 2018 10:49
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@dimitris-athanasiou
Copy link
Contributor

Would it be possible to add a YML test for this?

@droberts195
Copy link

Would it be possible to add a YML test for this?

The existing test already has undefined behaviour without this change. It sometimes fails due to the two events with the same start time having indeterminate order. For example, see https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+intake/2019/console

Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@davidkyle davidkyle merged commit 16d1f05 into elastic:master Jun 4, 2018
@davidkyle davidkyle deleted the events-sort branch June 4, 2018 15:37
davidkyle added a commit that referenced this pull request Jun 4, 2018
davidkyle added a commit that referenced this pull request Jun 4, 2018
dnhatn added a commit that referenced this pull request Jun 4, 2018
* master:
  Match phrase queries against non-indexed fields should throw an exception (#31060)
  In the internal highlighter APIs, use the field type as opposed to the mapper. (#31039)
  [DOCS] Removes duplicated authentication pages
  Enable customizing REST tests blacklist (#31074)
  Make sure KeywordFieldMapper#clone preserves split_queries_on_whitespace. (#31049)
  [DOCS] Moves machine learning overview to stack-docs
  [ML] Add secondary sort to ML events (#31063)
  [Rollup] Specialize validation exception for easier management (#30339)
  Adapt bwc versions after backporting #31045 to 6.3
  Remove usage of explicit type in docs (#29667)
  Share common readFrom/writeTo code in AcknowledgeResponse (#30983)
  Adapt bwc versions after backporting #31045 to 6.x
  Mute MatchPhrase*QueryBuilderTests
  [Docs] Fix typo in watcher conditions documentation (#30989)
  Remove wrong link in index phrases doc
  Move pipeline APIs to ingest namespace (#31027)
  [DOCS] Fixes accounting setting names (#30863)
  [DOCS] Rewords _field_names documentation (#31029)
  Index phrases (#30450)
  Remove leftover debugging from PTCMDT
  Fix PTCMDT#testMinVersionSerialization
  Make Persistent Tasks implementations version and feature aware (#31045)
dnhatn added a commit that referenced this pull request Jun 4, 2018
* 6.x:
  Add TRACE, CONNECT, and PATCH http methods (#31079)
  Change ObjectParser exception (#31030)
  Make sure KeywordFieldMapper#clone preserves split_queries_on_whitespace. (#31049)
  [DOCS] Removes duplicated authentication pages
  [Rollup] Specialize validation exception for easier management (#30339)
  Enable customizing REST tests blacklist (#31074)
  [DOCS] Moves machine learning overview to stack-docs
  [ML] Add secondary sort to ML events (#31063)
  QA: Check rollup job creation safety (#31036)
  Adapt bwc versions after backporting #31045 to 6.3
  Remove usage of explicit type in docs (#29667)
  Move pipeline APIs to ingest namespace (#31027)
  Adapt bwc versions after backporting #31045 to 6.x
  Make Persistent Tasks implementations version and feature aware (#31045)
  Mute MatchPhrase*QueryBuilderTests
  [Docs] Fix typo in watcher conditions documentation (#30989)
  Remove wrong link in index phrases doc
  Reuse expiration date of trial licenses (#31033)
  Index phrases (#30450)
  [DOCS] Fixes accounting setting names (#30863)
  [DOCS] Rewords _field_names documentation (#31029)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:ml Machine learning >non-issue >test Issues or PRs that are addressing/adding tests v6.3.0 v6.4.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants