forked from elastic/kibana
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[search source] remove is_partial check (elastic#164506)
Closes elastic#164893 ### Background "is partial" has 2 meanings 1) Results are incomplete because search is still running 2) Search is finished. Results are incomplete because there are shard failures (either in local or remote clusters) [async search](https://www.elastic.co/guide/en/elasticsearch/reference/current/async-search.html) defines 2 flags. 1) `is_running`: Whether the search is still being executed or it has completed 2) `is_partial`: When the query is no longer running, indicates whether the search failed or was successfully completed on all shards. While the query is being executed, is_partial is always set to true **note**: there is a bug in async search where requests to only local clusters return `is_partial:false` when there are shard errors on the local cluster. See elastic/elasticsearch#98725. This should be resolved in 8.11 Kibana's existing search implementation does not align with Elasticsearch's `is_running` and `is_partial` flags. Kibana defines "is partial" as definition "1)". Elasticsearch async search defines "is partial" as definition "2)". This PR aligns Kibana's usage of "is partial" with Elasticsearch's definition. This required the following changes 1) `isErrorResponse` renamed to `isAbortedResponse`. Method no longer returns true when `!response.isRunning && !!response.isPartial`. Kibana handles results with incomplete data. **Note** removed export of `isErrorResponse` from data plugin since its use outside of data plugin does not make sense. 2) Replace `isPartialResponse` with `isRunningResponse`. This aligns Kibana's definition with Elasticsearch async search flags. 3) Remove `isCompleteResponse`. The word "complete" is ambiguous. Does it mean the search is finished (no longer running)? Or does it mean the search has all results and there are no shard failures? --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Jatin Kathuria <[email protected]> Co-authored-by: Patryk Kopyciński <[email protected]>
- Loading branch information
4 people
authored
Oct 2, 2023
1 parent
236eff4
commit b5bcf69
Showing
47 changed files
with
192 additions
and
441 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,210 +6,42 @@ | |
| * Side Public License, v 1. | ||
| */ | ||
|
|
||
| import { isErrorResponse, isCompleteResponse, isPartialResponse } from './utils'; | ||
| import type { IKibanaSearchResponse } from './types'; | ||
| import { isAbortResponse, isRunningResponse } from './utils'; | ||
|
|
||
| describe('utils', () => { | ||
| describe('isErrorResponse', () => { | ||
| describe('isAbortResponse', () => { | ||
| it('returns `true` if the response is undefined', () => { | ||
| const isError = isErrorResponse(); | ||
| const isError = isAbortResponse(); | ||
| expect(isError).toBe(true); | ||
| }); | ||
|
|
||
| it('returns `true` if the response is not running and partial', () => { | ||
| const isError = isErrorResponse({ | ||
| isPartial: true, | ||
| isRunning: false, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(true); | ||
| }); | ||
|
|
||
| it('returns `false` if the response is not running and partial and contains failure details', () => { | ||
| const isError = isErrorResponse({ | ||
| isPartial: true, | ||
| isRunning: false, | ||
| rawResponse: { | ||
| took: 7, | ||
| timed_out: false, | ||
| _shards: { | ||
| total: 2, | ||
| successful: 1, | ||
| skipped: 0, | ||
| failed: 1, | ||
| failures: [ | ||
| { | ||
| shard: 0, | ||
| index: 'remote:tmp-00002', | ||
| node: '9SNgMgppT2-6UHJNXwio3g', | ||
| reason: { | ||
| type: 'script_exception', | ||
| reason: 'runtime error', | ||
| script_stack: [ | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.getFactoryForDoc(LeafDocLookup.java:148)', | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:191)', | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:32)', | ||
| "doc['bar'].value < 10", | ||
| ' ^---- HERE', | ||
| ], | ||
| script: "doc['bar'].value < 10", | ||
| lang: 'painless', | ||
| position: { | ||
| offset: 4, | ||
| start: 0, | ||
| end: 21, | ||
| }, | ||
| caused_by: { | ||
| type: 'illegal_argument_exception', | ||
| reason: 'No field found for [bar] in mapping', | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| _clusters: { | ||
| total: 1, | ||
| successful: 1, | ||
| skipped: 0, | ||
| details: { | ||
| remote: { | ||
| status: 'partial', | ||
| indices: 'tmp-*', | ||
| took: 3, | ||
| timed_out: false, | ||
| _shards: { | ||
| total: 2, | ||
| successful: 1, | ||
| skipped: 0, | ||
| failed: 1, | ||
| }, | ||
| failures: [ | ||
| { | ||
| shard: 0, | ||
| index: 'remote:tmp-00002', | ||
| node: '9SNgMgppT2-6UHJNXwio3g', | ||
| reason: { | ||
| type: 'script_exception', | ||
| reason: 'runtime error', | ||
| script_stack: [ | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.getFactoryForDoc(LeafDocLookup.java:148)', | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:191)', | ||
| '[email protected]/org.elasticsearch.search.lookup.LeafDocLookup.get(LeafDocLookup.java:32)', | ||
| "doc['bar'].value < 10", | ||
| ' ^---- HERE', | ||
| ], | ||
| script: "doc['bar'].value < 10", | ||
| lang: 'painless', | ||
| position: { | ||
| offset: 4, | ||
| start: 0, | ||
| end: 21, | ||
| }, | ||
| caused_by: { | ||
| type: 'illegal_argument_exception', | ||
| reason: 'No field found for [bar] in mapping', | ||
| }, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }, | ||
| hits: { | ||
| total: { | ||
| value: 1, | ||
| relation: 'eq', | ||
| }, | ||
| max_score: 0, | ||
| hits: [ | ||
| { | ||
| _index: 'remote:tmp-00001', | ||
| _id: 'd8JNlYoBFqAcOBVnvdqx', | ||
| _score: 0, | ||
| _source: { | ||
| foo: 'bar', | ||
| bar: 1, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }); | ||
| expect(isError).toBe(false); | ||
| }); | ||
|
|
||
| it('returns `false` if the response is running and partial', () => { | ||
| const isError = isErrorResponse({ | ||
| isPartial: true, | ||
| isRunning: true, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(false); | ||
| }); | ||
|
|
||
| it('returns `false` if the response is complete', () => { | ||
| const isError = isErrorResponse({ | ||
| isPartial: false, | ||
| isRunning: false, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isCompleteResponse', () => { | ||
| it('returns `false` if the response is undefined', () => { | ||
| const isError = isCompleteResponse(); | ||
| expect(isError).toBe(false); | ||
| }); | ||
|
|
||
| it('returns `false` if the response is running and partial', () => { | ||
| const isError = isCompleteResponse({ | ||
| isPartial: true, | ||
| isRunning: true, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(false); | ||
| }); | ||
|
|
||
| it('returns `true` if the response is complete', () => { | ||
| const isError = isCompleteResponse({ | ||
| isPartial: false, | ||
| isRunning: false, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(true); | ||
| }); | ||
|
|
||
| it('returns `true` if the response does not indicate isRunning', () => { | ||
| const isError = isCompleteResponse({ | ||
| rawResponse: {}, | ||
| }); | ||
| it('returns `true` if rawResponse is undefined', () => { | ||
| const isError = isAbortResponse({} as unknown as IKibanaSearchResponse); | ||
| expect(isError).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe('isPartialResponse', () => { | ||
| describe('isRunningResponse', () => { | ||
| it('returns `false` if the response is undefined', () => { | ||
| const isError = isPartialResponse(); | ||
| expect(isError).toBe(false); | ||
| const isRunning = isRunningResponse(); | ||
| expect(isRunning).toBe(false); | ||
| }); | ||
|
|
||
| it('returns `true` if the response is running and partial', () => { | ||
| const isError = isPartialResponse({ | ||
| isPartial: true, | ||
| it('returns `true` if the response is running', () => { | ||
| const isRunning = isRunningResponse({ | ||
| isRunning: true, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(true); | ||
| expect(isRunning).toBe(true); | ||
| }); | ||
|
|
||
| it('returns `false` if the response is complete', () => { | ||
| const isError = isPartialResponse({ | ||
| isPartial: false, | ||
| it('returns `false` if the response is finished running', () => { | ||
| const isRunning = isRunningResponse({ | ||
| isRunning: false, | ||
| rawResponse: {}, | ||
| }); | ||
| expect(isError).toBe(false); | ||
| expect(isRunning).toBe(false); | ||
| }); | ||
| }); | ||
| }); | ||
Oops, something went wrong.