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

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

Merged
merged 4 commits into from
Jun 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -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`,
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,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 });
Expand Down
20 changes: 7 additions & 13 deletions x-pack/plugins/snapshot_restore/server/routes/api/repositories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -136,7 +130,7 @@ export function registerRepositoriesRoutes({
},
isManagedRepository: managedRepository === name,
snapshots: {
count: snapshots ? snapshots.length : null,
count: snapshotList ? snapshotList.length : null,
},
},
});
Expand Down
Loading