Make tie-breaking field configurable

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.
elastic · Jun 2, 2017 · 022f971 · 022f971
1 parent f054b53
commit 022f971
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 24 deletions.
diff --git a/src/core_plugins/kibana/public/context/api/__tests__/anchor.js b/src/core_plugins/kibana/public/context/api/__tests__/anchor.js
@@ -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);
         });
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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' },
           ]);
         });
     });
@@ -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');
@@ -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);

diff --git a/src/core_plugins/kibana/public/context/api/anchor.js b/src/core_plugins/kibana/public/context/api/anchor.js
@@ -19,7 +19,7 @@ function fetchAnchorProvider(courier, Private) {
           _uid: [uid],
         },
       })
-      .set('sort', [sort, { _uid: 'asc' }]);
+      .set('sort', sort);
 
     const response = await searchSource.fetch();
 

diff --git a/src/core_plugins/kibana/public/context/api/context.js b/src/core_plugins/kibana/public/context/api/context.js
@@ -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,
     );
@@ -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,

diff --git a/src/core_plugins/kibana/public/context/api/utils/sorting.js b/src/core_plugins/kibana/public/context/api/utils/sorting.js
@@ -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.
  *
@@ -72,6 +100,7 @@ function reverseSortDirection(sortDirection) {
 
 
 export {
+  getFirstSortableField,
   reverseSortDirection,
   reverseSortDirective,
 };
diff --git a/src/core_plugins/kibana/public/context/app.js b/src/core_plugins/kibana/public/context/app.js
@@ -4,6 +4,7 @@ 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,
@@ -52,6 +53,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,
   );
 
@@ -111,9 +113,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,

diff --git a/src/core_plugins/kibana/public/context/query/actions.js b/src/core_plugins/kibana/public/context/query/actions.js
@@ -26,12 +26,12 @@ export function QueryActionsProvider(courier, Notifier, Private, Promise) {
   );
 
   const fetchAnchorRow = (state) => () => {
-    const { queryParameters: { indexPatternId, anchorUid, sort } } = state;
+    const { queryParameters: { indexPatternId, anchorUid, sort, tieBreakerField } } = state;
 
     setLoadingStatus(state)('anchor', LOADING_STATUS.LOADING);
 
     return Promise.try(() => (
-      fetchAnchor(indexPatternId, anchorUid, _.zipObject([sort]))
+      fetchAnchor(indexPatternId, anchorUid, [_.zipObject([sort]), { [tieBreakerField]: 'asc' }])
     ))
       .then(
         (anchorDocument) => {
@@ -49,14 +49,14 @@ export function QueryActionsProvider(courier, Notifier, Private, Promise) {
 
   const fetchPredecessorRows = (state) => () => {
     const {
-      queryParameters: { indexPatternId, filters, predecessorCount, sort },
+      queryParameters: { indexPatternId, filters, predecessorCount, sort, tieBreakerField },
       rows: { anchor },
     } = state;
 
     setLoadingStatus(state)('predecessors', LOADING_STATUS.LOADING);
 
     return Promise.try(() => (
-      fetchPredecessors(indexPatternId, anchor, _.zipObject([sort]), predecessorCount, filters)
+      fetchPredecessors(indexPatternId, anchor, [_.zipObject([sort]), { [tieBreakerField]: 'asc' }], predecessorCount, filters)
     ))
       .then(
         (predecessorDocuments) => {
@@ -74,14 +74,14 @@ export function QueryActionsProvider(courier, Notifier, Private, Promise) {
 
   const fetchSuccessorRows = (state) => () => {
     const {
-      queryParameters: { indexPatternId, filters, sort, successorCount },
+      queryParameters: { indexPatternId, filters, sort, successorCount, tieBreakerField },
       rows: { anchor },
     } = state;
 
     setLoadingStatus(state)('successors', LOADING_STATUS.LOADING);
 
     return Promise.try(() => (
-      fetchSuccessors(indexPatternId, anchor, _.zipObject([sort]), successorCount, filters)
+      fetchSuccessors(indexPatternId, anchor, [_.zipObject([sort]), { [tieBreakerField]: 'asc' }], successorCount, filters)
     ))
       .then(
         (successorDocuments) => {

diff --git a/src/core_plugins/kibana/public/context/query_parameters/state.js b/src/core_plugins/kibana/public/context/query_parameters/state.js
@@ -1,4 +1,4 @@
-export function createInitialQueryParametersState(defaultStepSize) {
+export function createInitialQueryParametersState(defaultStepSize, tieBreakerField) {
   return {
     anchorUid: null,
     columns: [],
@@ -8,5 +8,6 @@ export function createInitialQueryParametersState(defaultStepSize) {
     predecessorCount: 0,
     successorCount: 0,
     sort: [],
+    tieBreakerField,
   };
 }
diff --git a/src/ui/ui_settings/defaults.js b/src/ui/ui_settings/defaults.js
@@ -341,6 +341,12 @@ export function getDefaultSettings() {
     'context:step': {
       value: 5,
       description: 'The step size to increment or decrement the context size by',
-    }
+    },
+    'context:tieBreakerFields': {
+      value: ['_doc'],
+      description: 'A list of fields to use for tie-breaking 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.',
+    },
   };
 }