-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Support pit
and search_after
in server savedObjects.find
#89915
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments after taking a first look
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
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Outdated
Show resolved
Hide resolved
5eb5577
to
943bfe3
Compare
Pinging @elastic/kibana-core (Team:Core) |
@elasticmachine merge upstream (most of the functional test failures were due to #88737 not being merged yet) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Show resolved
Hide resolved
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Show resolved
Hide resolved
x-pack/plugins/security/server/saved_objects/secure_saved_objects_client_wrapper.ts
Show resolved
Hide resolved
x-pack/plugins/spaces/server/saved_objects/spaces_saved_objects_client.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppServices change LGTM.
a8c9699
to
ea6305f
Compare
pit
and search_after
in savedObjects.find
pit
and search_after
in savedObjects.find
on the server
BRB, need to go get a tattoo. 😄 |
pit
and search_after
in savedObjects.find
on the serverpit
and search_after
in server savedObjects.find
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Backport result
|
@lukeelmers how do I test this please? Thanks! |
@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. |
… 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
Closes #86301
Summary
Adds support for both
pit
andsearch_after
to 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'smaxImportExportSize
config value).Changes
searchAfter
option to SO.findpit
option to SO.findpit_id
to SearchResponseopenPointInTimeForType
to 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)closePointInTime
for cleaning up a PIT via SO client (without requiring you to use the Elasticsearch service)openPointInTimeForType
andclosePointInTime
max_result_window
default of10000
.PointInTimeFinder
which returns a generator to make paging through results easier. Plan is to eventually expose this as a public helper.sort
value with a tiebreaker fieldUsage
With
find
Finder helper
Follow-up tasks (will create a separate issue)
maxImportExportSize
maxImportExportSize
to be number instead of bytes (which is incorrect)