[Discover] Experimental usage of ES fields API #75407
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -256,7 +256,7 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise | |
| $scope.state = { ...newState }; | ||
|
|
||
| // detect changes that should trigger fetching of new data | ||
| const changes = ['interval', 'sort', 'columns'].filter( | ||
|
There was a problem hiding this comment. need to resubmit the query to display subfields like There was a problem hiding this comment. I am a bit concerned around performance of that part. Right now we're not doing additional queries (to fetch data) if we modify the columns. Especially given that the Discover query is atm very unoptimized (e.g. contains the aggregation for the chart in each fetch for the data), and thus can potentially also be rather slow-ish, doing this now for every column add/remove/reorder feels like a significant performance regression (e.g. on our issue instance, which is not having a significant large dataset, every discover request takes ~2s). Especially when a user starts building a discover view, she'll need to first add a couple of columns, which will then all trigger another request and we could slow down the creation time and UX significantly with this change. We could optimize this code to e.g. exclude column reorders, but I have the feeling we're still in a problematic situation afterwards. I currently don't see how we could check if a change to column would require us to refetch, since we're not knowing which of the column fields could be runtime fields and thus require a refetch here. I am happy for any suggestions here. cc @stacey-gammon There was a problem hiding this comment.
Yes, this is the disadvantage of this solution, but we since we currently don't know which fields are runtime, we would currently need to fetch all fields + _source as an alternative.
Wouldn't each new fetch abort the old one? There was a problem hiding this comment.
I think we need to evaluate and try to quantify the fallout of this change a bit better. We should not make this kind of larger performance impacts in the most uses application in Kibana willy-nilly and hopef or the best.
That would miticate the problem a bit, but without the quantification I have the feeling we still run into problems, since if we assume a 2 second request, this could correlate very roughly to the interaction time the user needs to get to the next field they want to add, so they actually never run into the situation where the previous request would be cancelled, but into a situation where the UI every time they want to add the next column is frozen for some time, because we're parsing some large JSON response and redrawing the page. |
||
| (prop) => !_.isEqual(newStatePartial[prop], oldStatePartial[prop]) | ||
| ); | ||
|
|
||
|
|
@@ -950,6 +950,8 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise | |
| const { indexPattern, searchSource } = $scope; | ||
| searchSource | ||
| .setField('index', $scope.indexPattern) | ||
| .setField('fields', $scope.state.columns) | ||
| .setField('source', true) | ||
| .setField('size', $scope.opts.sampleSize) | ||
| .setField( | ||
| 'sort', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dear @jtibshirani , with this change Discover via the underlying search service would use the fields API, and would therefore support runtime fields. Question is: could there be fields available in
stored_fieldsthat aren't in thefieldsAPI? thx!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kertal
stored_fieldsare not included in the 'fields' option. So yes, there could be fields available instored_fieldsthat do not come back infields.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jtibshirani thx, i've solved it differently now!