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

[SOM] Fix UI and export when handling more than 10k objects #118335

Merged

Conversation

pgayvallet
Copy link
Contributor

@pgayvallet pgayvallet commented Nov 11, 2021

Summary

Fix #116776

  • Adapt the /api/kibana/management/saved_objects/scroll/counts endpoint to use PIT search
  • Adapt the SOM UI to improve experience when displaying more than 10k objects
    • limit the results in the table to 10k
    • when more than 10k results are returned, display a message stating that we're only displaying the first 10k results
  • Improve the toast error message displayed to the user when the export fail by displaying the error message as returned by the server
  • (cleanup) Remove the /api/kibana/management/saved_objects/scroll/export endpoint that was unused (was used by the legacy export that was removed long ago)

Screenshots

Displaying 12k vis

Screenshot 2021-11-11 at 14 20 59

Toast notification in case of failure

Screenshot 2021-11-11 at 14 28 30

Checklist

  • Documentation was added for features that require explanation or tutorials

Release Note

Fix a bug in the saved objects management section causing the export of more than 10k objects to fail.

@pgayvallet pgayvallet added bug Fixes for quality problems that affect the customer experience Feature:Saved Objects Management Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v8.0.0 v8.1.0 release_note:fix labels Nov 11, 2021
Copy link
Contributor Author

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

self-review

Comment on lines +106 to +110
for (let i = startIdx; i <= endIdx; i++) {
const id = `test-vis-${i}`;
fileChunks.push(
JSON.stringify({
type: 'visualization',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially created a file with 10k entries with a script and used kibanaServer.importExport API, but the file is quite big and the API only accepts uncompressed files.

Also, this is how this is done for the export and import test suites too:

fileChunks.push(
JSON.stringify({
type: 'visualization',
id: `${SPACE_ID}-${i}`,
attributes: {

Comment on lines +129 to +130
await importVisualizations({ startIdx: 1, endIdx: 6000 });
await importVisualizations({ startIdx: 6001, endIdx: 12000 });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the import limit is 10001, I'm imported the 12k in two batches. This is also an additional check that the scroll_count API works when there are more objects than the limit in the search results.

Comment on lines +97 to +100
describe('scroll_count with more than 10k objects', () => {
const importVisualizations = async ({
startIdx = 1,
endIdx,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only added FTR tests for this API because export and import were already properly working and are covered, e.g

it('should allow exporting more than 10,000 objects if permitted by maxImportExportSize', async () => {
await supertest
.post(`/s/${SPACE_ID}/api/saved_objects/_export`)

@@ -74,6 +74,8 @@ interface TableState {
activeAction?: SavedObjectsManagementAction;
}

const MAX_PAGINATED_ITEM = 10000;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a big fan of this hardcoded limit, OTOH that's how it's done in other parts of Kibana too, and it's way easier than fetching the setting from all the SO indices targeted by the request.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can an end-user face the problem when requesting /api/kibana/management/saved_objects/scroll/counts? If so, maybe this logic belongs to the server-side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can't when requesting /api/kibana/management/saved_objects/scroll/counts, or when performing the export, only when manually calling /api/kibana/management/saved_objects/_find if the requested page/perPage exceeds the threshold (but even if prefixed by /api, there shouldn't be a case where an integration uses this endpoint)

@pgayvallet pgayvallet marked this pull request as ready for review November 11, 2021 16:37
@pgayvallet pgayvallet requested a review from a team as a code owner November 11, 2021 16:37
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@pgayvallet
Copy link
Contributor Author

@elasticmachine merge upstream

@elastic elastic deleted a comment from kibanamachine Nov 12, 2021
@@ -74,6 +74,8 @@ interface TableState {
activeAction?: SavedObjectsManagementAction;
}

const MAX_PAGINATED_ITEM = 10000;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can an end-user face the problem when requesting /api/kibana/management/saved_objects/scroll/counts? If so, maybe this logic belongs to the server-side?

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
savedObjectsManagement 82.0KB 82.6KB +648.0B

History

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

@pgayvallet pgayvallet added the auto-backport Deprecated - use backport:version if exact versions are needed label Nov 16, 2021
@pgayvallet pgayvallet merged commit 42957d9 into elastic:main Nov 16, 2021
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Nov 16, 2021
…118335)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
8.0

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Nov 16, 2021
…#118669)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Pierre Gayvallet <[email protected]>
pgayvallet added a commit to pgayvallet/kibana that referenced this pull request Nov 16, 2021
…118335)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/plugins/saved_objects_management/server/routes/scroll_export.ts
pgayvallet added a commit that referenced this pull request Nov 16, 2021
…#118718)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/plugins/saved_objects_management/server/routes/scroll_export.ts
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 19, 2021
…118335)

* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>
dmlemeshko pushed a commit that referenced this pull request Nov 29, 2021
* delete the unused `scroll/export` endpoint

* adapt the scroll/counts endpoint to use PIT search

* improve export error message

* only display 10k first items

* fix findAll unit tests

* fix FTR test

* remove unused comment

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed bug Fixes for quality problems that affect the customer experience Feature:Saved Objects Management release_note:fix Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.16.0 v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saved objects management paging fails if paging past 10k saved objects
5 participants