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

Background search service POC #64641

Closed
wants to merge 102 commits into from

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Apr 28, 2020

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@lizozom lizozom self-assigned this Apr 28, 2020
@@ -23,4 +23,6 @@ import { CoreSetup, SharedGlobalConfig } from '../../../../core/server';
export interface ISearchContext {
core: CoreSetup;
config$: Observable<SharedGlobalConfig>;
backgroundSearchService?: any;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

How to extend context??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -139,6 +140,15 @@ export class SearchSource {
return { ...this.fields };
}

getSessionId(): string | undefined {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be private

const response = await context.search!.search(
searchRequest,
{ signal, rawRequest: request },
strategy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should strategy be a part of options?

Copy link
Member

Choose a reason for hiding this comment

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

FWIW it would make sense as an option to me instead of its own argument (and uses the default strategy if left empty), but 🤷‍♂️

const user = context?.security?.authc.getCurrentUser(options.rawRequest);
if (user) {
context.backgroundSearchService.trackId(
user.email,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is user.email the best info to use (in terms of uniqueness)?

@@ -71,14 +106,16 @@ async function asyncSearch(
const path = encodeURI(request.id ? `/_async_search/${request.id}` : `/${index}/_async_search`);

// Wait up to 1s for the response to return
const query = toSnakeCase({ waitForCompletionTimeout: '1s', ...queryParams });
const query = toSnakeCase({ waitForCompletionTimeout: '1ms', ...queryParams });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revert me!

@lizozom lizozom requested a review from lukasolson April 28, 2020 12:19
@lizozom lizozom added the v8.0.0 label Apr 30, 2020
@lizozom
Copy link
Contributor Author

lizozom commented Apr 30, 2020

@elasticmachine merge upstream

@@ -98,7 +83,7 @@ export class SearchInterceptor {
): Observable<IEsSearchResponse> {
const { id, ...searchRequest } = request;
const path = trimEnd(`/internal/search/${strategy || ES_SEARCH_STRATEGY}/${id || ''}`, '/');
const body = JSON.stringify(id != null ? {} : searchRequest);
const body = JSON.stringify(id != null ? { stored: request.stored } : searchRequest);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Need to make sure that searchRequest has the correct shape, rather than making these decidions in the OSS interceptor

@kibanamachine
Copy link
Contributor

kibanamachine commented Aug 17, 2020

💔 Build Failed

Failed CI Steps


Test Failures

X-Pack Jest Tests.x-pack/plugins/data_enhanced/public/search.EnhancedSearchInterceptor search should resolve immediately if first call returns full result

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

TypeError: this.deps.session.getStored is not a function
    at EnhancedSearchInterceptor.search (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts:80:40)
    at Object.test (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts:103:42)
    at Promise (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:198:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:162:10)
    at _callCircusTest (/dev/shm/workspace/kibana/node_modules/jest-circus/build/run.js:205:40)
    at process._tickCallback (internal/process/next_tick.js:68:7)

X-Pack Jest Tests.x-pack/plugins/data_enhanced/public/search.EnhancedSearchInterceptor search should make secondary request if first call returns partial result

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

TypeError: this.deps.session.getStored is not a function
    at EnhancedSearchInterceptor.search (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts:80:40)
    at Object.test (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts:141:42)
    at Promise (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:198:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:162:10)
    at _callCircusTest (/dev/shm/workspace/kibana/node_modules/jest-circus/build/run.js:205:40)
    at process._tickCallback (internal/process/next_tick.js:68:7)

X-Pack Jest Tests.x-pack/plugins/data_enhanced/public/search.EnhancedSearchInterceptor search should abort if request is partial and not running (ES graceful error)

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches


Stack Trace

TypeError: this.deps.session.getStored is not a function
    at EnhancedSearchInterceptor.search (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.ts:80:40)
    at Object.test (/dev/shm/workspace/parallel/10/kibana/x-pack/plugins/data_enhanced/public/search/search_interceptor.test.ts:172:42)
    at Promise (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:198:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/dev/shm/workspace/kibana/node_modules/jest-circus/build/utils.js:162:10)
    at _callCircusTest (/dev/shm/workspace/kibana/node_modules/jest-circus/build/run.js:205:40)
    at process._tickCallback (internal/process/next_tick.js:68:7)

and 12 more failures, only showing the first 3.

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
data 545 +5 540
dataEnhanced 175 +157 18
total +162

page load bundle size

id value diff baseline
dashboard 689.7KB +2.0KB 687.7KB
data 1.4MB +4.6KB 1.4MB
dataEnhanced 604.9KB ⚠️ +426.9KB 178.0KB
total +433.5KB

Saved Objects .kibana field count

id value diff baseline
background-session 8 +8 -

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@lizozom lizozom closed this Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants