Support pit and search_after in server savedObjects.find#89915
Merged
lukeelmers merged 37 commits intoelastic:masterfrom Feb 11, 2021
Merged
Support pit and search_after in server savedObjects.find#89915lukeelmers merged 37 commits intoelastic:masterfrom
pit and search_after in server savedObjects.find#89915lukeelmers merged 37 commits intoelastic:masterfrom
Conversation
pgayvallet
reviewed
Feb 2, 2021
Contributor
pgayvallet
left a comment
There was a problem hiding this comment.
Some comments after taking a first look
src/core/server/saved_objects/service/lib/repository_es_client.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/service/lib/search_dsl/search_dsl.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Outdated
Show resolved
Hide resolved
f5b8aad to
056bc0b
Compare
c3dfcf1 to
5eb5577
Compare
lukeelmers
commented
Feb 7, 2021
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Outdated
Show resolved
Hide resolved
5eb5577 to
943bfe3
Compare
Contributor
|
Pinging @elastic/kibana-core (Team:Core) |
rudolf
reviewed
Feb 8, 2021
Contributor
Author
|
@elasticmachine merge upstream (most of the functional test failures were due to #88737 not being merged yet) |
legrego
reviewed
Feb 9, 2021
Member
legrego
left a comment
There was a problem hiding this comment.
First pass through, mostly questions and nits
src/core/server/saved_objects/export/find_with_point_in_time.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/export/find_with_point_in_time.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/export/find_with_point_in_time.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/export/find_with_point_in_time.ts
Outdated
Show resolved
Hide resolved
src/core/server/saved_objects/export/find_with_point_in_time.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/server/authorization/privileges/privileges.test.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts
Outdated
Show resolved
Hide resolved
streamich
approved these changes
Feb 9, 2021
Contributor
streamich
left a comment
There was a problem hiding this comment.
AppServices change LGTM.
a8c9699 to
ea6305f
Compare
pit and search_after in savedObjects.findpit and search_after in savedObjects.find on the server
Contributor
Author
BRB, need to go get a tattoo. 😄 |
pit and search_after in savedObjects.find on the serverpit and search_after in server savedObjects.find
This was referenced Feb 11, 2021
Contributor
Author
Contributor
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Contributor
|
Backport result |
This was referenced Feb 11, 2021
lukeelmers
added a commit
that referenced
this pull request
Feb 11, 2021
This was referenced Feb 19, 2021
45 tasks
Contributor
|
@lukeelmers how do I test this please? Thanks! |
Contributor
Author
|
@bhavyarm Ah, sorry - forgot to include testing notes:
Also worth mentioning that we do have some functional tests here that verify exporting >10k saved objects works. |
1 task
FrankHassanabad
added a commit
that referenced
this pull request
Feb 15, 2022
… from saved objects to exception lists (#125182) ## Summary Exposes the functionality of * search_after * point in time (pit) From saved objects to the exception lists. This _DOES NOT_ expose these to the REST API just yet. Rather this exposes it at the API level to start with and changes code that had hard limits of 10k and other limited loops. I use the batching of 1k for this at a time as I thought that would be a decent batch guess and I see other parts of the code changed to it. It's easy to change the 1k if we find we need to throttle back more as we get feedback from others. See this PR where `PIT` and `search_after` were first introduced: #89915 See these 2 issues where we should be using more paging and PIT (Point in Time) with search_after: #93770 #103944 The new methods added to the `exception_list_client.ts` client class are: * openPointInTime * closePointInTime * findExceptionListItemPointInTimeFinder * findExceptionListPointInTimeFinder * findExceptionListsItemPointInTimeFinder * findValueListExceptionListItemsPointInTimeFinder The areas of functionality that have been changed: * Exception list exports * Deletion of lists * Getting exception list items when generating signals Note that currently we use our own ways of looping over the saved objects which you can see in the codebase such as this older way below which does work but had a limitation of 10k against saved objects and did not do point in time (PIT) Older way example (deprecated): ```ts let page = 1; let ids: string[] = []; let foundExceptionListItems = await findExceptionListItem({ filter: undefined, listId, namespaceType, page, perPage: PER_PAGE, pit: undefined, savedObjectsClient, searchAfter: undefined, sortField: 'tie_breaker_id', sortOrder: 'desc', }); while (foundExceptionListItems != null && foundExceptionListItems.data.length > 0) { ids = [ ...ids, ...foundExceptionListItems.data.map((exceptionListItem) => exceptionListItem.id), ]; page += 1; foundExceptionListItems = await findExceptionListItem({ filter: undefined, listId, namespaceType, page, perPage: PER_PAGE, pit: undefined, savedObjectsClient, searchAfter: undefined, sortField: 'tie_breaker_id', sortOrder: 'desc', }); } return ids; ``` But now that is replaced with this newer way using PIT: ```ts // Stream the results from the Point In Time (PIT) finder into this array let ids: string[] = []; const executeFunctionOnStream = (response: FoundExceptionListItemSchema): void => { const responseIds = response.data.map((exceptionListItem) => exceptionListItem.id); ids = [...ids, ...responseIds]; }; await findExceptionListItemPointInTimeFinder({ executeFunctionOnStream, filter: undefined, listId, maxSize: undefined, // NOTE: This is unbounded when it is "undefined" namespaceType, perPage: 1_000, savedObjectsClient, sortField: 'tie_breaker_id', sortOrder: 'desc', }); return ids; ``` We also have areas of code that has perPage listed at 10k or a constant that represents 10k which this removes in most areas (but not all areas): ```ts const items = await client.findExceptionListsItem({ listId: listIds, namespaceType: namespaceTypes, page: 1, pit: undefined, perPage: MAX_EXCEPTION_LIST_SIZE, // <--- Really bad to send in 10k per page at a time searchAfter: undefined, filter: [], sortOrder: undefined, sortField: undefined, }); ``` That is now: ```ts // Stream the results from the Point In Time (PIT) finder into this array let items: ExceptionListItemSchema[] = []; const executeFunctionOnStream = (response: FoundExceptionListItemSchema): void => { items = [...items, ...response.data]; }; await client.findExceptionListsItemPointInTimeFinder({ executeFunctionOnStream, listId: listIds, namespaceType: namespaceTypes, perPage: 1_000, filter: [], maxSize: undefined, // NOTE: This is unbounded when it is "undefined" sortOrder: undefined, sortField: undefined, }); ``` Left over areas will be handled in separate PR's because they are in other people's code ownership areas. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #86301
Summary
Adds support for both
pitandsearch_afterto Saved Objects on the server, enabling folks to reliably page through sets of results even if exceeding theindex.max_result_window(or more accurately, if exceeding Kibana'smaxImportExportSizeconfig value).Changes
searchAfteroption to SO.findpitoption to SO.findpit_idto SearchResponseopenPointInTimeForTypeto SO client, which allows you to pass a type (or list of types), and have a PIT opened against the index/indices backing the type(s)closePointInTimefor cleaning up a PIT via SO client (without requiring you to use the Elasticsearch service)openPointInTimeForTypeandclosePointInTimemax_result_windowdefault of10000.PointInTimeFinderwhich returns a generator to make paging through results easier. Plan is to eventually expose this as a public helper.sortvalue with a tiebreaker fieldUsage
With
findFinder helper
Follow-up tasks (will create a separate issue)
maxImportExportSizemaxImportExportSizeto be number instead of bytes (which is incorrect)