Skip to content

Commit

Permalink
[context view] Use _doc for tie-breaking instead of _uid (#12096)
Browse files Browse the repository at this point in the history
Using fields with docvalues (like `_doc`) for tie-breaking yields
significantly better performance than using `_uid`, which lacks
docvalues at the moment. The downside is that sorting by `_doc` by
default is not stable under all conditions, but better than no
tie-breaking at all.

The new setting `context:tieBreakingFields` enables the user to
customize the list of fields Kibana attempts to use for tie-breaking.
The first field from that list, that is sortable in the current index
pattern, will be used. It defaults to `_doc`, which should change to
`_seq_no` from version 6.0 on.

In addition to just showing a notification, errors that occur while
loading documents from the database will be stored as part of the
`loadingStatus` along with a reason code (if known). This is used to
display more nuanced and helpful error messages to the user.

The first such error message indicates a missing or invalid tiebreaker
field required for sorting the context.
  • Loading branch information
weltenwort authored Jun 8, 2017
1 parent a271d7c commit a2727ec
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 50 deletions.
1 change: 1 addition & 0 deletions docs/management/advanced-options.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,4 @@ Markdown.
`state:storeInSessionStorage`:: [experimental] Kibana tracks UI state in the URL, which can lead to problems when there is a lot of information there and the URL gets very long. Enabling this will store parts of the state in your browser session instead, to keep the URL shorter.
`context:defaultSize`:: Specifies the initial number of surrounding entries to display in the context view. The default value is 5.
`context:step`:: Specifies the number to increment or decrement the context size by when using the buttons in the context view. The default value is 5.
`context:tieBreakerFields`:: A comma-separated list of fields to use for tiebreaking between documents that have the same timestamp value. From this list the first field that is present and sortable in the current index pattern is used.
20 changes: 10 additions & 10 deletions src/core_plugins/kibana/public/context/api/__tests__/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('context app', function () {
it('should use the `fetch` method of the SearchSource', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
expect(searchSourceStub.fetch.calledOnce).to.be(true);
});
Expand All @@ -39,7 +39,7 @@ describe('context app', function () {
it('should configure the SearchSource to not inherit from the implicit root', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const inheritsSpy = searchSourceStub.inherits;
expect(inheritsSpy.calledOnce).to.be(true);
Expand All @@ -50,7 +50,7 @@ describe('context app', function () {
it('should set the SearchSource index pattern', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const setIndexSpy = searchSourceStub.set.withArgs('index');
expect(setIndexSpy.calledOnce).to.be(true);
Expand All @@ -61,7 +61,7 @@ describe('context app', function () {
it('should set the SearchSource version flag to true', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const setVersionSpy = searchSourceStub.set.withArgs('version');
expect(setVersionSpy.calledOnce).to.be(true);
Expand All @@ -72,7 +72,7 @@ describe('context app', function () {
it('should set the SearchSource size to 1', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const setSizeSpy = searchSourceStub.set.withArgs('size');
expect(setSizeSpy.calledOnce).to.be(true);
Expand All @@ -83,7 +83,7 @@ describe('context app', function () {
it('should set the SearchSource query to a _uid terms query', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const setQuerySpy = searchSourceStub.set.withArgs('query');
expect(setQuerySpy.calledOnce).to.be(true);
Expand All @@ -98,13 +98,13 @@ describe('context app', function () {
it('should set the SearchSource sort order', function () {
const searchSourceStub = new SearchSourceStub();

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(() => {
const setSortSpy = searchSourceStub.set.withArgs('sort');
expect(setSortSpy.calledOnce).to.be(true);
expect(setSortSpy.firstCall.args[1]).to.eql([
{ '@timestamp': 'desc' },
{ '_uid': 'asc' },
{ '_doc': 'asc' },
]);
});
});
Expand All @@ -113,7 +113,7 @@ describe('context app', function () {
const searchSourceStub = new SearchSourceStub();
searchSourceStub._stubHits = [];

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then(
() => {
expect().fail('expected the promise to be rejected');
Expand All @@ -131,7 +131,7 @@ describe('context app', function () {
{ property2: 'value2' },
];

return fetchAnchor('INDEX_PATTERN_ID', 'UID', { '@timestamp': 'desc' })
return fetchAnchor('INDEX_PATTERN_ID', 'UID', [{ '@timestamp': 'desc' }, { '_doc': 'asc' }])
.then((anchorDocument) => {
expect(anchorDocument).to.have.property('property1', 'value1');
expect(anchorDocument).to.have.property('$$_isAnchor', true);
Expand Down
2 changes: 1 addition & 1 deletion src/core_plugins/kibana/public/context/api/anchor.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function fetchAnchorProvider(courier, Private) {
_uid: [uid],
},
})
.set('sort', [sort, { _uid: 'asc' }]);
.set('sort', sort);

const response = await searchSource.fetch();

Expand Down
5 changes: 2 additions & 3 deletions src/core_plugins/kibana/public/context/api/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@ function fetchContextProvider(courier, Private) {
};

async function fetchSuccessors(indexPatternId, anchorDocument, contextSort, size, filters) {
const successorsSort = [contextSort, { _uid: 'asc' }];
const successorsSearchSource = await createSearchSource(
indexPatternId,
anchorDocument,
successorsSort,
contextSort,
size,
filters,
);
Expand All @@ -27,7 +26,7 @@ function fetchContextProvider(courier, Private) {
}

async function fetchPredecessors(indexPatternId, anchorDocument, contextSort, size, filters) {
const predecessorsSort = [reverseSortDirective(contextSort), { _uid: 'desc' }];
const predecessorsSort = contextSort.map(reverseSortDirective);
const predecessorsSearchSource = await createSearchSource(
indexPatternId,
anchorDocument,
Expand Down
29 changes: 29 additions & 0 deletions src/core_plugins/kibana/public/context/api/utils/sorting.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
import _ from 'lodash';


/**
* The list of field names that are allowed for sorting, but not included in
* index pattern fields.
*
* @constant
* @type {string[]}
*/
const META_FIELD_NAMES = ['_seq_no', '_doc', '_uid'];

/**
* Returns a field from the intersection of the set of sortable fields in the
* given index pattern and a given set of candidate field names.
*
* @param {IndexPattern} indexPattern - The index pattern to search for
* sortable fields
* @param {string[]} fields - The list of candidate field names
*
* @returns {string[]}
*/
function getFirstSortableField(indexPattern, fieldNames) {
const sortableFields = fieldNames.filter((fieldName) => (
META_FIELD_NAMES.includes(fieldName)
|| (indexPattern.fields.byName[fieldName] || { sortable: false }).sortable
));
return sortableFields[0];
}

/**
* A sort directive in object or string form.
*
Expand Down Expand Up @@ -72,6 +100,7 @@ function reverseSortDirection(sortDirection) {


export {
getFirstSortableField,
reverseSortDirection,
reverseSortDirective,
};
40 changes: 27 additions & 13 deletions src/core_plugins/kibana/public/context/app.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,33 @@
<!-- Error feedback -->
<div
class="kuiViewContent kuiViewContentItem"
ng-if="contextApp.state.loadingStatus.anchor === contextApp.constants.LOADING_STATUS.FAILED"
ng-if="contextApp.state.loadingStatus.anchor.status === contextApp.constants.LOADING_STATUS.FAILED"
>
<div class="kuiInfoPanel kuiInfoPanel--error kuiVerticalRhythm">
<div class="kuiInfoPanelHeader">
<span class="kuiInfoPanelHeader__icon kuiIcon kuiIcon--error fa-warning"></span>
<span class="kuiInfoPanelHeader__title">
Problem with query
Failed to load the anchor document
</span>
</div>

<div class="kuiInfoPanelBody">
<div class="kuiInfoPanelBody__message">
Failed to load the anchor document. Please reload or visit
<div
class="kuiInfoPanelBody__message"
ng-if="contextApp.state.loadingStatus.anchor.reason === contextApp.constants.FAILURE_REASONS.INVALID_TIEBREAKER"
>
No searchable tiebreaker field could be found in the index pattern
{{ contextApp.state.queryParameters.indexPatternId}}.

Please change the advanced setting
<code>context:tieBreakerFields</code> to include a valid field for this
index pattern.
</div>
<div
class="kuiInfoPanelBody__message"
ng-if="contextApp.state.loadingStatus.anchor.reason === contextApp.constants.FAILURE_REASONS.UNKNOWN"
>
Please reload or visit
<a ng-href="{{ contextApp.state.navigation.discover.url }}">Discover</a>
to select a valid anchor document.
</div>
Expand All @@ -40,7 +54,7 @@

<div
class="kuiViewContent kuiViewContentItem"
ng-if="contextApp.state.loadingStatus.anchor !== contextApp.constants.LOADING_STATUS.FAILED"
ng-if="contextApp.state.loadingStatus.anchor.status !== contextApp.constants.LOADING_STATUS.FAILED"
role="main"
>
<!-- Controls -->
Expand All @@ -51,7 +65,7 @@
is-disabled="![
contextApp.constants.LOADING_STATUS.LOADED,
contextApp.constants.LOADING_STATUS.FAILED,
].includes(contextApp.state.loadingStatus.predecessors)"
].includes(contextApp.state.loadingStatus.predecessors.status)"
icon="'fa-chevron-up'"
ng-click="contextApp.actions.fetchMorePredecessorRows()"
>
Expand All @@ -60,13 +74,13 @@
<context-size-picker
count="contextApp.state.queryParameters.predecessorCount"
data-test-subj="predecessorCountPicker"
is-disabled="contextApp.state.loadingStatus.anchor !== contextApp.constants.LOADING_STATUS.LOADED"
is-disabled="contextApp.state.loadingStatus.anchor.status !== contextApp.constants.LOADING_STATUS.LOADED"
on-change-count="contextApp.actions.fetchGivenPredecessorRows"
></context-size-picker>
<span>newer documents</span>
<span
class="kuiStatusText kuiStatusText--warning"
ng-if="(contextApp.state.loadingStatus.predecessors === contextApp.constants.LOADING_STATUS.LOADED)
ng-if="(contextApp.state.loadingStatus.predecessors.status === contextApp.constants.LOADING_STATUS.LOADED)
&& (contextApp.state.queryParameters.predecessorCount > contextApp.state.rows.predecessors.length)"
>
<span class="kuiStatusText__icon kuiIcon fa-bolt"></span>
Expand All @@ -83,7 +97,7 @@
ng-if="[
contextApp.constants.LOADING_STATUS.UNINITIALIZED,
contextApp.constants.LOADING_STATUS.LOADING,
].includes(contextApp.state.loadingStatus.anchor)"
].includes(contextApp.state.loadingStatus.anchor.status)"
class="kuiPanel kuiPanel--centered kuiVerticalRhythm"
>
<div class="kuiTableInfo">
Expand All @@ -93,7 +107,7 @@

<!-- Table -->
<div
ng-if="contextApp.state.loadingStatus.anchor === contextApp.constants.LOADING_STATUS.LOADED"
ng-if="contextApp.state.loadingStatus.anchor.status === contextApp.constants.LOADING_STATUS.LOADED"
class="kuiPanel kuiVerticalRhythm"
>
<div class="discover-table" fixed-scroll>
Expand All @@ -116,7 +130,7 @@
is-disabled="![
contextApp.constants.LOADING_STATUS.LOADED,
contextApp.constants.LOADING_STATUS.FAILED,
].includes(contextApp.state.loadingStatus.successors)"
].includes(contextApp.state.loadingStatus.successors.status)"
icon="'fa-chevron-down'"
ng-click="contextApp.actions.fetchMoreSuccessorRows()"
>
Expand All @@ -125,13 +139,13 @@
<context-size-picker
count="contextApp.state.queryParameters.successorCount"
data-test-subj="successorCountPicker"
is-disabled="contextApp.state.loadingStatus.anchor !== contextApp.constants.LOADING_STATUS.LOADED"
is-disabled="contextApp.state.loadingStatus.anchor.status !== contextApp.constants.LOADING_STATUS.LOADED"
on-change-count="contextApp.actions.fetchGivenSuccessorRows"
></context-size-picker>
<div>older documents</div>
<span
class="kuiStatusText kuiStatusText--warning"
ng-if="(contextApp.state.loadingStatus.successors === contextApp.constants.LOADING_STATUS.LOADED)
ng-if="(contextApp.state.loadingStatus.successors.status === contextApp.constants.LOADING_STATUS.LOADED)
&& (contextApp.state.queryParameters.successorCount > contextApp.state.rows.successors.length)"
>
<span class="kuiStatusText__icon kuiIcon fa-bolt"></span>
Expand Down
8 changes: 6 additions & 2 deletions src/core_plugins/kibana/public/context/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import { uiModules } from 'ui/modules';
import contextAppTemplate from './app.html';
import './components/loading_button';
import './components/size_picker/size_picker';
import { getFirstSortableField } from './api/utils/sorting';
import {
createInitialQueryParametersState,
QueryParameterActionsProvider,
QUERY_PARAMETER_KEYS,
} from './query_parameters';
import {
createInitialLoadingStatusState,
FAILURE_REASONS,
LOADING_STATUS,
QueryActionsProvider,
} from './query';
Expand Down Expand Up @@ -52,6 +54,7 @@ function ContextAppController($scope, config, Private, timefilter) {

this.state = createInitialState(
parseInt(config.get('context:step'), 10),
getFirstSortableField(this.indexPattern, config.get('context:tieBreakerFields')),
this.discoverUrl,
);

Expand All @@ -62,6 +65,7 @@ function ContextAppController($scope, config, Private, timefilter) {
), (action) => (...args) => action(this.state)(...args));

this.constants = {
FAILURE_REASONS,
LOADING_STATUS,
};

Expand Down Expand Up @@ -111,9 +115,9 @@ function ContextAppController($scope, config, Private, timefilter) {
);
}

function createInitialState(defaultStepSize, discoverUrl) {
function createInitialState(defaultStepSize, tieBreakerField, discoverUrl) {
return {
queryParameters: createInitialQueryParametersState(defaultStepSize),
queryParameters: createInitialQueryParametersState(defaultStepSize, tieBreakerField),
rows: {
all: [],
anchor: null,
Expand Down
Loading

0 comments on commit a2727ec

Please sign in to comment.