Skip to content

Commit

Permalink
[Fleet] Remove duplicative retries from client-side requests to APIs …
Browse files Browse the repository at this point in the history
…that depend on EPR (elastic#190722)

## Summary

Relates to elastic#136617

For APIs that depend on Fleet connecting to Elastic Package Registry,
Fleet already retries the connections to EPR on the server side. This
results in a situation where, when EPR is unreachable, the requests is
retried several times on the server side, and then the request is
retried again on the client-side by react-query. This results in very
long running API requests.

Since the server-side retries generally cover any kind of flakiness
here, disabling the retry logic on the front-end seems sensible. ~I've
also reduced the number of retries on the server side from 5 to 3 to
help fail faster here.~ I walked back the retry change after some test
failures, and I don't think it makes a big enough impact to justify
changing.

## To test

Set `xpack.fleet.registryUrl: 127.0.0.1:8080` with nothing running

## Before

The requests spin for a very long time.


https://github.com/user-attachments/assets/e4fd77ee-b36c-4965-9f71-e5b3e195f75e

## After

The requests stop spinning after a few seconds as the retries won't
loop.


https://github.com/user-attachments/assets/82adc595-1bc4-4269-8501-2eb83525ad15

cc @shahzad31
  • Loading branch information
kpollich authored Aug 20, 2024
1 parent cd1d645 commit cf3149e
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,18 @@

import React, { useCallback, useRef, useEffect, forwardRef } from 'react';
import { css } from '@emotion/react';
import { EuiFlexGrid, EuiFlexItem, EuiSpacer, EuiText, EuiAutoSizer } from '@elastic/eui';
import {
EuiFlexGrid,
EuiFlexItem,
EuiSpacer,
EuiText,
EuiAutoSizer,
EuiSkeletonRectangle,
} from '@elastic/eui';
import { VariableSizeList as List } from 'react-window';
import { FormattedMessage } from '@kbn/i18n-react';
import { WindowScroller } from 'react-virtualized';

import { Loading } from '../../../../components';
import type { IntegrationCardItem } from '../../screens/home';

import { PackageCard } from '../package_card';
Expand Down Expand Up @@ -66,7 +72,17 @@ export const GridColumn = ({
}
}, []);

if (isLoading) return <Loading />;
if (isLoading) {
return (
<EuiFlexGrid gutterSize="l" columns={3}>
{Array.from({ length: 12 }).map((_, index) => (
<EuiFlexItem key={index} grow={3}>
<EuiSkeletonRectangle height="160px" width="100%" />
</EuiFlexItem>
))}
</EuiFlexGrid>
);
}

if (!list.length) {
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { EuiFacetButton, EuiFacetGroup } from '@elastic/eui';
import { EuiFacetButton, EuiFacetGroup, EuiSpacer } from '@elastic/eui';
import type { IntegrationCategory } from '@kbn/custom-integrations-plugin/common';
import React from 'react';

Expand Down Expand Up @@ -78,7 +78,11 @@ export function CategoryFacets({
const controls = (
<EuiFacetGroup>
{isLoading ? (
<Loading />
<>
<EuiSpacer size="m" />
<Loading size="l" />
<EuiSpacer size="m" />
</>
) : (
categories.map((category) => {
return (
Expand Down
30 changes: 22 additions & 8 deletions x-pack/plugins/fleet/public/hooks/use_request/epm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,16 @@ export function useGetReplacementCustomIntegrationsQuery() {
}

export function useGetCategoriesQuery(query: GetCategoriesRequest['query'] = {}) {
return useQuery<GetCategoriesResponse, RequestError>(['categories', query], () =>
sendRequestForRq<GetCategoriesResponse>({
path: epmRouteService.getCategoriesPath(),
method: 'get',
query,
version: API_VERSIONS.public.v1,
})
return useQuery<GetCategoriesResponse, RequestError>(
['categories', query],
() =>
sendRequestForRq<GetCategoriesResponse>({
path: epmRouteService.getCategoriesPath(),
method: 'get',
query,
version: API_VERSIONS.public.v1,
}),
{ retry: (_, error) => !isRegistryConnectionError(error), refetchOnWindowFocus: false }
);
}

Expand Down Expand Up @@ -96,6 +99,8 @@ export const useGetPackagesQuery = (
query,
}),
enabled: options?.enabled,
retry: (_, error) => !isRegistryConnectionError(error),
refetchOnWindowFocus: false,
});
};

Expand Down Expand Up @@ -151,7 +156,12 @@ export const useGetPackageInfoByKeyQuery = (
...(ignoreUnverifiedQueryParam && { ignoreUnverified: ignoreUnverifiedQueryParam }),
},
}),
{ enabled: queryOptions.enabled, refetchOnMount: queryOptions.refetchOnMount }
{
enabled: queryOptions.enabled,
refetchOnMount: queryOptions.refetchOnMount,
retry: (_, error) => !isRegistryConnectionError(error),
refetchOnWindowFocus: false,
}
);

const confirm = async () => {
Expand Down Expand Up @@ -360,3 +370,7 @@ export function useGetInputsTemplatesQuery(
})
);
}

function isRegistryConnectionError(error: RequestError) {
return error.statusCode === 502;
}

0 comments on commit cf3149e

Please sign in to comment.