[Snapshot + Restore] Set snapshots response size limit #103331

alisonelizabeth · 2021-06-24T17:32:24Z

This PR sets a limit (1000) on the number of snapshots displayed in the snapshots table in Snapshot + Restore and displays a warning if that limit is reached. This is a temporary solution to support scenarios where a user has a large number of snapshots that could potentially create performance issues on the client. In the near future, this will be replaced in favor of server-side pagination.

This PR is supported on the ES side by: elastic/elasticsearch#74451, elastic/elasticsearch#73952.

Also fixes #44817

How to test

I'm not sure if the ES changes are available via snapshot yet, so you may need to build from source: yarn es source -E path.repo=/tmp/es_backups.
Create a local FS repository. You will need to define the max_number_of_snapshots setting (default is 500). This setting isn't available in the UI, so it's easiest to set up via Console:

PUT /_snapshot/my_repo
{
  "type": "fs",
  "settings": {
    "location": "/tmp/es_backups",
    "max_number_of_snapshots": 1200
  }
}

Create >1000 snapshots. I've created a gist with instructions to create snapshots in bulk. This will take some time to run.
Visit the "Snapshots" tab in Snapshot + Restore. Verify the snapshots are still displayed as expected and warning displays.

Note on ES API changes
As part of this work, I've replaced the batched snapshot requests in favor of fetching all snapshots in a single request. The ES snapshots API should also now be compatible again on 8.0 and 7.x, and we will no longer need to maintain two different codebases 🎉 (original PR where divergence was introduced: #39533).

Screenshots

…mit_snapshots

elasticmachine · 2021-06-28T13:32:37Z

Pinging @elastic/kibana-stack-management (Team:Stack Management)

alisonelizabeth · 2021-06-28T13:33:19Z

/cc @henningandersen @original-brownbear

lockewritesdocs

Added some suggested revisions to the message text. Let me know if you want to discuss further!

...ck/plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx

…me/snapshot_list/snapshot_list.tsx Co-authored-by: Adam Locke <adam.locke@elastic.co>

cjcenizal

Thanks for doing this @alisonelizabeth! Tested locally, code LGTM.

cjcenizal · 2021-06-28T17:21:02Z

x-pack/plugins/snapshot_restore/common/constants.ts

@@ -65,3 +65,9 @@ export const TIME_UNITS: { [key: string]: 'd' | 'h' | 'm' | 's' } = {
  MINUTE: 'm',
  SECOND: 's',
 };
+
+/**
+ * [Temporary workaround] In order to prevent an unusable UI for users with a large number of snapshots,


Can we clarify what makes it unusable? For example, is it a client-side performance issue?

cjcenizal · 2021-06-28T17:39:39Z

x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts

@@ -116,7 +116,7 @@ export function registerRepositoriesRoutes({
        snapshot: '_all',
      });

-      const { responses: snapshotResponses } = response.body;
+      const { snapshots: snapshotList } = response.body;


As part of the ES changes, the snapshots API should now be compatible again on 8.0 and 7.x

Note for myself and anyone else who is curious. This looks like a breaking change but it's really bringing master into alignment with 7.x (https://github.com/elastic/kibana/blob/7.x/x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts#L115)

cjcenizal · 2021-06-28T18:45:59Z

...ck/plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx

+            >
+              <FormattedMessage
+                id="xpack.snapshotRestore.snapshotsList.maxSnapshotsDisplayedDescription"
+                defaultMessage="You've reached the maximum number of snapshots. To view all of your snapshots, use {docLink}."


@lockewritesdocs Does "You've reached the maximum number of snapshots" sound like you've hit a technical limit, and you can't have more snapshots beyond this point? I think something like "You've reached the maximum number of snapshots that can be displayed" will remove this ambiguity. WDYT?

Also, do you think it'd be helpful to mention what that limit is, somehow?

We could say something like, "You've reached the maximum number of viewable snapshots."

I'm debating whether the limit is useful. If it's 500 or it's 1,000, does it matter if I can't see them? I'm not entirely sure.

I've changed this to: "You've reached the maximum number of viewable snapshots." I'm not sure if the limit is useful either. I think it's also somewhat obvious based on the pagination.

++ @alisonelizabeth -- I think this is a great compromise.

cjcenizal

Didn't re-test locally but I re-reviewed the code and it's great to see we're retaining the "partial response" UX even if some of the snapshots have errors. Thanks for digging into this!

…mit_snapshots

kibanamachine · 2021-06-29T15:32:27Z

💚 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
`snapshotRestore`	460.2KB	461.6KB	+1.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	417.7KB	417.7KB	+55.0B
`snapshotRestore`	41.1KB	41.2KB	+118.0B
total			+173.0B

Unknown metric groups

API count

id	before	after	diff
`snapshotRestore`	22	23	+1

History

💚 Build #134904 succeeded e9e1604
💚 Build #134785 succeeded 9012397
💚 Build #134740 succeeded cf414bd
💔 Build #134732 failed b05ccd3
💛 Build #134575 was flaky db15ad3

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

#103693) * [Snapshot + Restore] Set snapshots response size limit (#103331) # Conflicts: # x-pack/plugins/snapshot_restore/server/routes/api/repositories.test.ts # x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts # x-pack/plugins/snapshot_restore/server/routes/api/snapshots.test.ts # x-pack/plugins/snapshot_restore/server/routes/api/snapshots.ts * fix merge conflict * skip test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Use single ES request to fetch snapshots + set response size limit

015c443

alisonelizabeth added 4 commits June 24, 2021 13:48

remove unused translations

5e5b641

add warning callout

a1800ea

increase max limit; add tests

1f2725c

Merge branch 'master' of https://github.com/elastic/kibana into sr/li…

db15ad3

…mit_snapshots

alisonelizabeth marked this pull request as ready for review June 28, 2021 13:32

alisonelizabeth requested review from a team as code owners June 28, 2021 13:32

alisonelizabeth requested a review from cjcenizal June 28, 2021 13:32

pgayvallet approved these changes Jun 28, 2021

View reviewed changes

lockewritesdocs reviewed Jun 28, 2021

View reviewed changes

...ck/plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx Outdated Show resolved Hide resolved

...ck/plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx Outdated Show resolved Hide resolved

alisonelizabeth and others added 3 commits June 28, 2021 13:49

Update x-pack/plugins/snapshot_restore/public/application/sections/ho…

6afde62

…me/snapshot_list/snapshot_list.tsx Co-authored-by: Adam Locke <adam.locke@elastic.co>

Update x-pack/plugins/snapshot_restore/public/application/sections/ho…

b05ccd3

…me/snapshot_list/snapshot_list.tsx Co-authored-by: Adam Locke <adam.locke@elastic.co>

update tests

cf414bd

cjcenizal approved these changes Jun 28, 2021

View reviewed changes

alisonelizabeth added 3 commits June 28, 2021 15:31

address review feedback

9012397

add back support for repository errors

c4c8e6f

add back warning prompt

e9e1604

cjcenizal approved these changes Jun 29, 2021

View reviewed changes

Merge branch 'master' of https://github.com/elastic/kibana into sr/li…

2bfd00f

…mit_snapshots

alisonelizabeth merged commit e387d3d into elastic:master Jun 29, 2021

alisonelizabeth deleted the sr/limit_snapshots branch June 29, 2021 15:32

This was referenced Jun 29, 2021

[7.x] [Snapshot + Restore] Set snapshots response size limit (#103331) #103693

Merged

[Snapshot + Restore] Fix error handling for repositories API #103723

Merged

yuliacech mentioned this pull request Jul 5, 2021

[SLM] Testing UI with large number of snapshots? #56839

Closed

This was referenced Jul 13, 2021

[Snapshot and Restore] Snapshots tab does not show instructions for creating first snapshot #105385

Closed

[Snapshots + Restore] Fix no snapshots prompt #105460

Merged

yuliacech mentioned this pull request Sep 21, 2021

[Snapshot and Restore] Server side snapshots pagination #110266

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Snapshot + Restore] Set snapshots response size limit #103331

[Snapshot + Restore] Set snapshots response size limit #103331

alisonelizabeth commented Jun 24, 2021 •

edited

Loading

elasticmachine commented Jun 28, 2021

alisonelizabeth commented Jun 28, 2021

lockewritesdocs left a comment

cjcenizal left a comment

cjcenizal Jun 28, 2021

cjcenizal Jun 28, 2021

cjcenizal Jun 28, 2021

lockewritesdocs Jun 28, 2021

alisonelizabeth Jun 28, 2021

lockewritesdocs Jun 28, 2021

cjcenizal left a comment

kibanamachine commented Jun 29, 2021

API count

[Snapshot + Restore] Set snapshots response size limit #103331

[Snapshot + Restore] Set snapshots response size limit #103331

Conversation

alisonelizabeth commented Jun 24, 2021 • edited Loading

How to test

Screenshots

elasticmachine commented Jun 28, 2021

alisonelizabeth commented Jun 28, 2021

lockewritesdocs left a comment

Choose a reason for hiding this comment

cjcenizal left a comment

Choose a reason for hiding this comment

cjcenizal Jun 28, 2021

Choose a reason for hiding this comment

cjcenizal Jun 28, 2021

Choose a reason for hiding this comment

cjcenizal Jun 28, 2021

Choose a reason for hiding this comment

lockewritesdocs Jun 28, 2021

Choose a reason for hiding this comment

alisonelizabeth Jun 28, 2021

Choose a reason for hiding this comment

lockewritesdocs Jun 28, 2021

Choose a reason for hiding this comment

cjcenizal left a comment

Choose a reason for hiding this comment

kibanamachine commented Jun 29, 2021

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

API count

History

alisonelizabeth commented Jun 24, 2021 •

edited

Loading