Skip to content

Commit

Permalink
[Snapshot + Restore] Set snapshots response size limit (#103331)
Browse files Browse the repository at this point in the history
  • Loading branch information
alisonelizabeth authored Jun 29, 2021
1 parent 2f4b9f5 commit e387d3d
Show file tree
Hide file tree
Showing 14 changed files with 223 additions and 131 deletions.
1 change: 1 addition & 0 deletions src/core/public/doc_links/doc_links_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,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`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,4 +379,7 @@ export type TestSubjects =
| 'verifyRepositoryButton'
| 'version'
| 'version.title'
| 'version.value';
| 'version.value'
| 'maxSnapshotsWarning'
| 'repositoryErrorsWarning'
| 'repositoryErrorsPrompt';
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -461,7 +471,6 @@ describe('<SnapshotRestoreHome />', () => {
httpRequestsMockHelpers.setLoadSnapshotsResponse({
snapshots,
repositories: [REPOSITORY_NAME],
errors: {},
});

testBed = await setup();
Expand Down Expand Up @@ -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;

Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/snapshot_restore/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[]
Expand All @@ -33,6 +32,7 @@ export function deserializeSnapshotDetails(
const {
snapshot,
uuid,
repository,
version_id: versionId,
version,
indices = [],
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/snapshot_restore/common/types/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface SnapshotDetails {
export interface SnapshotDetailsEs {
snapshot: string;
uuid: string;
repository: string;
version_id: number;
version: string;
indices: string[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -319,7 +320,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
);
} else {
const repositoryErrorsWarning = Object.keys(errors).length ? (
<Fragment>
<>
<EuiCallOut
title={
<FormattedMessage
Expand All @@ -329,6 +330,7 @@ export const SnapshotList: React.FunctionComponent<RouteComponentProps<MatchPara
}
color="warning"
iconType="alert"
data-test-subj="repositoryErrorsWarning"
>
<FormattedMessage
id="xpack.snapshotRestore.repositoryWarningDescription"
Expand All @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,7 @@ describe('[Snapshot and Restore API Routes] Repositories', () => {
[name]: { type: '', settings: {} },
};
const mockEsSnapshotResponse = {
responses: [
{
repository: name,
snapshots: [{}, {}],
},
],
snapshots: [{ repository: name }, { repository: name }],
};

getClusterSettingsFn.mockResolvedValue({ body: mockSnapshotGetManagedRepositoryEsResponse });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export function registerRepositoriesRoutes({
snapshot: '_all',
});

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

if (repositoryByName[name]) {
const { type = '', settings = {} } = repositoryByName[name];
Expand All @@ -130,10 +130,7 @@ export function registerRepositoriesRoutes({
},
isManagedRepository: managedRepository === name,
snapshots: {
count:
snapshotResponses && snapshotResponses[0] && snapshotResponses[0].snapshots
? snapshotResponses[0].snapshots.length
: null,
count: snapshotList ? snapshotList.length : null,
},
},
});
Expand Down
Loading

0 comments on commit e387d3d

Please sign in to comment.