[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
elastic · Jun 29, 2021 · 208e463 · 208e463
1 parent b38fd69
commit 208e463
Show file tree

Hide file tree

Showing 14 changed files with 232 additions and 118 deletions.
diff --git a/src/core/public/doc_links/doc_links_service.ts b/src/core/public/doc_links/doc_links_service.ts
@@ -357,6 +357,7 @@ export class DocLinksService {
           guide: `${KIBANA_DOCS}snapshot-repositories.html`,
           changeIndexSettings: `${ELASTICSEARCH_DOCS}snapshots-restore-snapshot.html#change-index-settings-during-restore`,
           createSnapshot: `${ELASTICSEARCH_DOCS}snapshots-take-snapshot.html`,
+          getSnapshot: `${ELASTICSEARCH_DOCS}get-snapshot-api.html`,
           registerSharedFileSystem: `${ELASTICSEARCH_DOCS}snapshots-register-repository.html#snapshots-filesystem-repository`,
           registerSourceOnly: `${ELASTICSEARCH_DOCS}snapshots-register-repository.html#snapshots-source-only-repository`,
           registerUrl: `${ELASTICSEARCH_DOCS}snapshots-register-repository.html#snapshots-read-only-repository`,

diff --git a/x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/home.helpers.ts b/x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/home.helpers.ts
@@ -379,4 +379,7 @@ export type TestSubjects =
   | 'verifyRepositoryButton'
   | 'version'
   | 'version.title'
-  | 'version.value';
+  | 'version.value'
+  | 'maxSnapshotsWarning'
+  | 'repositoryErrorsWarning'
+  | 'repositoryErrorsPrompt';
diff --git a/x-pack/plugins/snapshot_restore/__jest__/client_integration/home.test.ts b/x-pack/plugins/snapshot_restore/__jest__/client_integration/home.test.ts
@@ -33,6 +33,16 @@ jest.mock('@kbn/i18n/react', () => {
   };
 });
 
+jest.mock('../../common/constants', () => {
+  const original = jest.requireActual('../../common/constants');
+
+  return {
+    ...original,
+    // Mocking this value to a lower number in order to more easily trigger the max snapshots warning in the tests
+    SNAPSHOT_LIST_MAX_SIZE: 2,
+  };
+});
+
 const removeWhiteSpaceOnArrayValues = (array: any[]) =>
   array.map((value) => {
     if (!value.trim) {
@@ -461,7 +471,6 @@ describe('<SnapshotRestoreHome />', () => {
         httpRequestsMockHelpers.setLoadSnapshotsResponse({
           snapshots,
           repositories: [REPOSITORY_NAME],
-          errors: {},
         });
 
         testBed = await setup();
@@ -494,6 +503,69 @@ describe('<SnapshotRestoreHome />', () => {
         });
       });
 
+      test('should show a warning if the number of snapshots exceeded the limit', () => {
+        // We have mocked the SNAPSHOT_LIST_MAX_SIZE to 2, so the warning should display
+        const { find, exists } = testBed;
+        expect(exists('maxSnapshotsWarning')).toBe(true);
+        expect(find('maxSnapshotsWarning').text()).toContain(
+          'Cannot show the full list of snapshots'
+        );
+      });
+
+      test('should show a warning if one repository contains errors', async () => {
+        httpRequestsMockHelpers.setLoadSnapshotsResponse({
+          snapshots,
+          repositories: [REPOSITORY_NAME],
+          errors: {
+            repository_with_errors: {
+              type: 'repository_exception',
+              reason:
+                '[repository_with_errors] Could not read repository data because the contents of the repository do not match its expected state.',
+            },
+          },
+        });
+
+        testBed = await setup();
+
+        await act(async () => {
+          testBed.actions.selectTab('snapshots');
+        });
+
+        testBed.component.update();
+
+        const { find, exists } = testBed;
+        expect(exists('repositoryErrorsWarning')).toBe(true);
+        expect(find('repositoryErrorsWarning').text()).toContain(
+          'Some repositories contain errors'
+        );
+      });
+
+      test('should show a prompt if a repository contains errors and there are no other repositories', async () => {
+        httpRequestsMockHelpers.setLoadSnapshotsResponse({
+          snapshots,
+          repositories: [],
+          errors: {
+            repository_with_errors: {
+              type: 'repository_exception',
+              reason:
+                '[repository_with_errors] Could not read repository data because the contents of the repository do not match its expected state.',
+            },
+          },
+        });
+
+        testBed = await setup();
+
+        await act(async () => {
+          testBed.actions.selectTab('snapshots');
+        });
+
+        testBed.component.update();
+
+        const { find, exists } = testBed;
+        expect(exists('repositoryErrorsPrompt')).toBe(true);
+        expect(find('repositoryErrorsPrompt').text()).toContain('Some repositories contain errors');
+      });
+
       test('each row should have a link to the repository', async () => {
         const { component, find, exists, table, router } = testBed;
 

diff --git a/x-pack/plugins/snapshot_restore/common/constants.ts b/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 client-side performance issues for users with a large number of snapshots,
+ * we set a hard-coded limit on the number of snapshots we return from the ES snapshots API
+ */
+export const SNAPSHOT_LIST_MAX_SIZE = 1000;
diff --git a/x-pack/plugins/snapshot_restore/common/lib/snapshot_serialization.test.ts b/x-pack/plugins/snapshot_restore/common/lib/snapshot_serialization.test.ts
@@ -12,10 +12,10 @@ describe('Snapshot serialization and deserialization', () => {
     test('deserializes a snapshot', () => {
       expect(
         deserializeSnapshotDetails(
-          'repositoryName',
           {
             snapshot: 'snapshot name',
             uuid: 'UUID',
+            repository: 'repositoryName',
             version_id: 5,
             version: 'version',
             indices: ['index2', 'index3', 'index1'],
@@ -55,6 +55,7 @@ describe('Snapshot serialization and deserialization', () => {
             {
               snapshot: 'last_successful_snapshot',
               uuid: 'last_successful_snapshot_UUID',
+              repository: 'repositoryName',
               version_id: 5,
               version: 'version',
               indices: ['index2', 'index3', 'index1'],

diff --git a/x-pack/plugins/snapshot_restore/common/lib/snapshot_serialization.ts b/x-pack/plugins/snapshot_restore/common/lib/snapshot_serialization.ts
@@ -21,7 +21,6 @@ import { deserializeTime, serializeTime } from './time_serialization';
 import { csvToArray } from './utils';
 
 export function deserializeSnapshotDetails(
-  repository: string,
   snapshotDetailsEs: SnapshotDetailsEs,
   managedRepository?: string,
   successfulSnapshots?: SnapshotDetailsEs[]
@@ -33,6 +32,7 @@ export function deserializeSnapshotDetails(
   const {
     snapshot,
     uuid,
+    repository,
     version_id: versionId,
     version,
     indices = [],

diff --git a/x-pack/plugins/snapshot_restore/common/types/snapshot.ts b/x-pack/plugins/snapshot_restore/common/types/snapshot.ts
@@ -52,6 +52,7 @@ export interface SnapshotDetails {
 export interface SnapshotDetailsEs {
   snapshot: string;
   uuid: string;
+  repository: string;
   version_id: number;
   version: string;
   indices: string[];

diff --git a/...plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx b/...plugins/snapshot_restore/public/application/sections/home/snapshot_list/snapshot_list.tsx
@@ -19,7 +19,7 @@ import {
   EuiIcon,
 } from '@elastic/eui';
 
-import { APP_SLM_CLUSTER_PRIVILEGES } from '../../../../../common';
+import { APP_SLM_CLUSTER_PRIVILEGES, SNAPSHOT_LIST_MAX_SIZE } from '../../../../../common';
 import { WithPrivileges, PageLoading, PageError, Error } from '../../../../shared_imports';
 import { BASE_PATH, UIM_SNAPSHOT_LIST_LOAD } from '../../../constants';
 import { useLoadSnapshots } from '../../../services/http';
@@ -54,7 +54,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
     resendRequest: reload,
   } = useLoadSnapshots();
 
-  const { uiMetricService } = useServices();
+  const { uiMetricService, i18n } = useServices();
   const { docLinks } = useCore();
 
   const openSnapshotDetailsUrl = (
@@ -138,6 +138,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
       <EuiPageContent verticalPosition="center" horizontalPosition="center" color="danger">
         <EuiEmptyPrompt
           iconType="managementApp"
+          data-test-subj="repositoryErrorsPrompt"
           title={
             <h1 data-test-subj="title">
               <FormattedMessage
@@ -319,7 +320,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
     );
   } else {
     const repositoryErrorsWarning = Object.keys(errors).length ? (
-      <Fragment>
+      <>
         <EuiCallOut
           title={
             <FormattedMessage
@@ -329,6 +330,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
           }
           color="warning"
           iconType="alert"
+          data-test-subj="repositoryErrorsWarning"
         >
           <FormattedMessage
             id="xpack.snapshotRestore.repositoryWarningDescription"
@@ -346,13 +348,48 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
           />
         </EuiCallOut>
         <EuiSpacer />
-      </Fragment>
+      </>
     ) : null;
 
+    const maxSnapshotsWarning = snapshots.length === SNAPSHOT_LIST_MAX_SIZE && (
+      <>
+        <EuiCallOut
+          color="warning"
+          iconType="help"
+          data-test-subj="maxSnapshotsWarning"
+          title={i18n.translate('xpack.snapshotRestore.snapshotsList.maxSnapshotsDisplayedTitle', {
+            defaultMessage: 'Cannot show the full list of snapshots',
+          })}
+        >
+          <FormattedMessage
+            id="xpack.snapshotRestore.snapshotsList.maxSnapshotsDisplayedDescription"
+            defaultMessage="You've reached the maximum number of viewable snapshots. To view all of your snapshots, use {docLink}."
+            values={{
+              docLink: (
+                <EuiLink
+                  href={docLinks.links.snapshotRestore.getSnapshot}
+                  target="_blank"
+                  data-test-subj="documentationLink"
+                >
+                  <FormattedMessage
+                    id="xpack.snapshotRestore.snapshotsList.maxSnapshotsDisplayedDocLinkText"
+                    defaultMessage="the Elasticsearch API"
+                  />
+                </EuiLink>
+              ),
+            }}
+          />
+        </EuiCallOut>
+        <EuiSpacer size="l" />
+      </>
+    );
+
     content = (
       <section data-test-subj="snapshotList">
         {repositoryErrorsWarning}
 
+        {maxSnapshotsWarning}
+
         <SnapshotTable
           snapshots={snapshots}
           repositories={repositories}

diff --git a/x-pack/plugins/snapshot_restore/server/routes/api/repositories.test.ts b/x-pack/plugins/snapshot_restore/server/routes/api/repositories.test.ts
@@ -166,7 +166,7 @@ describe('[Snapshot and Restore API Routes] Repositories', () => {
         [name]: { type: '', settings: {} },
       };
       const mockEsSnapshotResponse = {
-        snapshots: [{}, {}],
+        snapshots: [{ repository: name }, { repository: name }],
       };
 
       getClusterSettingsFn.mockResolvedValue({ body: mockSnapshotGetManagedRepositoryEsResponse });

diff --git a/x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts b/x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts
@@ -111,18 +111,12 @@ export function registerRepositoriesRoutes({
         return handleEsError({ error: e, response: res });
       }
 
-      const {
-        body: { snapshots },
-      } = await clusterClient.asCurrentUser.snapshot
-        .get({
-          repository: name,
-          snapshot: '_all',
-        })
-        .catch((e) => ({
-          body: {
-            snapshots: null,
-          },
-        }));
+      const response = await clusterClient.asCurrentUser.snapshot.get({
+        repository: name,
+        snapshot: '_all',
+      });
+
+      const { snapshots: snapshotList } = response.body;
 
       if (repositoryByName[name]) {
         const { type = '', settings = {} } = repositoryByName[name];
@@ -136,7 +130,7 @@ export function registerRepositoriesRoutes({
             },
             isManagedRepository: managedRepository === name,
             snapshots: {
-              count: snapshots ? snapshots.length : null,
+              count: snapshotList ? snapshotList.length : null,
             },
           },
         });