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

Ability to disable highlighting #5197

Merged
merged 3 commits into from
Oct 28, 2015

Conversation

Filirom1
Copy link
Contributor

Hi,

When storing very big documents in ElasticSearch, highliting make requests very slow.
In my case, the same request with highlighting takes 76s and without highlighting 3s.

This is the reason why I would like to disable highlighting in some cases.

Cheers

@elasticsearch-release
Copy link

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run; then say 'jenkins, test it'.

@Filirom1 Filirom1 changed the title highlighting could be disabled Ability to disable highlighting Oct 26, 2015
@@ -136,6 +136,7 @@ define(function (require) {
index: $scope.indexPattern.id,
timefield: $scope.indexPattern.timeFieldName,
savedSearch: savedSearch,
highlight: config.get('discover:highlight'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Attaching this to $scope.opts is not necessary, this value isn't used outside of the immediate context.

@rashidkpc
Copy link
Contributor

I like the functionality. Please see comments.

@Filirom1
Copy link
Contributor Author

Ok thanks, I am working on it

@spalger
Copy link
Contributor

spalger commented Oct 26, 2015

Does this affect highlighting on the dashboard?

@Filirom1
Copy link
Contributor Author

No, this PR only affects the discover page.
I didn't have any issue with the Dashboard, I think because the table aggregation only print choosen fields and not the whole document.

@spalger
Copy link
Contributor

spalger commented Oct 26, 2015

I'm talking specifically about search sources rendered on the dashboard. I believe that the highlighting settings applied in discover is what causes highlighting to work when a searchSource is rendered as a panel.

@rashidkpc
Copy link
Contributor

This would affect both discover and the dashboard. The parameter should probably be namespaced to the doc table

@Filirom1
Copy link
Contributor Author

It affects both discover and the dashboard (saved searches)

@@ -59,6 +59,10 @@ define(function (require) {
value: 500,
description: 'The number of rows to show in the table',
},
'discover:highlight': {
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this affects both discover and dashboard, this should be doc_table:highlight

@Filirom1
Copy link
Contributor Author

Done

@rashidkpc
Copy link
Contributor

LGTM, going into 4.3.0

rashidkpc added a commit that referenced this pull request Oct 28, 2015
@rashidkpc rashidkpc merged commit f1e5418 into elastic:master Oct 28, 2015
@Filirom1
Copy link
Contributor Author

Thanks

@tbragin tbragin mentioned this pull request Nov 3, 2015
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants