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

[Discover] Use tiebreaker sort in main discover view #32426

Closed

Conversation

wylieconlon
Copy link
Contributor

Summary

When timestamps are equal, the tiebreaker field should be used to provide a consistent sort order. This field was being used only in the Context view, but not in the Discover view.

To reproduce this issue, go to Advanced Settings > Discover and set a tiebreaker field like offset,_doc.

Before:

screenshot 2019-03-04 15 58 55

After:

screenshot 2019-03-04 15 59 20

Closes #32221

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

@wylieconlon wylieconlon added release_note:enhancement Feature:Discover Discover Application Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Mar 4, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@wylieconlon
Copy link
Contributor Author

@chrisdavies Could you take a look at this when you get a chance?

Copy link
Contributor

@chrisdavies chrisdavies left a comment

Choose a reason for hiding this comment

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

LGTM. Just a review. Didn't pull down and test.

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

LGTM, when the conflicts are solved, this should be merged. tested locally with chrome

@kertal kertal self-requested a review June 18, 2019 13:26
Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

When the case with an empty tiebreaker is handled, conflicts are resolved, this should be merged

.setField('sort', [
getSort($state.sort, $scope.indexPattern),
{
[tiebreakerField]: defaultSortOrder,
Copy link
Member

Choose a reason for hiding this comment

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

No tiebreakerField (I've set the field to empty in Advanced Configuration) leads to the following sort {"undefined":{"order":"desc","unmapped_type":"boolean"}.

@Bargs
Copy link
Contributor

Bargs commented Jul 31, 2019

This isn't a bug, it was intentional. The tiebreaker was needed in the context app because it uses search_after so we needed a guaranteed sort order. Discover has no such requirement so we don't use it there. Applying this setting in Discover doesn't really hurt anything, but it does make the sort code slightly more complex for no real benefit IMO. Also the setting is specifically called "context:tieBreakerFields", so if we're going to use it in Discover we should probably update the name of the setting and the description in the docs.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@benbuzbee
Copy link

benbuzbee commented Sep 27, 2019

+1 for this. It would offer a pretty reasonable workaround for elastic/beats/7559

Use case; Using discover to analyze logs uploaded by filebeat which have millisecond collisions. Saved searches are a workaround but an extra-click barrier and less sharable. When using filebeat on indices using log.offset as a secondary sort field by default (or tiebreaker) seems to make a lot of sense.

@elasticmachine
Copy link
Contributor

💔 Build Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Discover Discover Application release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discover does not use tiebreaker sort order
6 participants