From 97890cff0ef9e7537b9f3859fdbff04661d80ad1 Mon Sep 17 00:00:00 2001 From: Walter Rafelsberger Date: Fri, 5 Nov 2021 13:51:21 +0100 Subject: [PATCH] [ML] Use abort signal instead of isCancelledRef. --- ..._failed_transactions_correlations.test.tsx | 2 - .../use_failed_transactions_correlations.ts | 78 ++++++++++--------- .../correlations/use_latency_correlations.ts | 20 ++--- 3 files changed, 46 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx index f4252e2d95554..929cc4f7f4cd3 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx +++ b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx @@ -167,8 +167,6 @@ describe('useFailedTransactionsCorrelations', () => { jest.advanceTimersByTime(100); await waitFor(() => expect(result.current.progress.loaded).toBe(0)); jest.advanceTimersByTime(100); - await waitFor(() => expect(result.current.progress.loaded).toBe(0)); - jest.advanceTimersByTime(100); await waitFor(() => expect(result.current.progress.loaded).toBe(0.05)); expect(result.current.progress).toEqual({ diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts index 2aa9134b96126..163223e744a22 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts +++ b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts @@ -52,17 +52,11 @@ export function useFailedTransactionsCorrelations() { [] ); - // `abortCtrl` is used to cancel individual requests that already started. - // `isCancelledRef` is used to cancel the overall task in between requests in the `startFetch` callback. const abortCtrl = useRef(new AbortController()); - // We're using a ref here because otherwise the startFetch function might have - // a stale value for checking if the task has been cancelled. - const isCancelledRef = useRef(false); const startFetch = useCallback(async () => { abortCtrl.current.abort(); abortCtrl.current = new AbortController(); - isCancelledRef.current = false; setResponse({ ...getInitialResponse(), @@ -85,36 +79,46 @@ export function useFailedTransactionsCorrelations() { ccsWarning: false, }; - // Initial call to fetch the overall distribution for the log-log plot. - const { overallHistogram, percentileThresholdValue } = await callApmApi({ - endpoint: 'POST /internal/apm/latency/overall_distribution', - signal: abortCtrl.current.signal, - params: { - body: { - ...fetchParams, - percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, - }, - }, - }); - responseUpdate.overallHistogram = overallHistogram; - responseUpdate.percentileThresholdValue = percentileThresholdValue; + const [overallHistogramResponse, errorHistogramRespone] = + await Promise.all([ + // Initial call to fetch the overall distribution for the log-log plot. + callApmApi({ + endpoint: 'POST /internal/apm/latency/overall_distribution', + signal: abortCtrl.current.signal, + params: { + body: { + ...fetchParams, + percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, + }, + }, + }), + callApmApi({ + endpoint: 'POST /internal/apm/latency/overall_distribution', + signal: abortCtrl.current.signal, + params: { + body: { + ...fetchParams, + percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, + termFilters: [ + { + fieldName: EVENT_OUTCOME, + fieldValue: EventOutcome.failure, + }, + ], + }, + }, + }), + ]); + + const { overallHistogram, percentileThresholdValue } = + overallHistogramResponse; + const { overallHistogram: errorHistogram } = errorHistogramRespone; - const { overallHistogram: errorHistogram } = await callApmApi({ - endpoint: 'POST /internal/apm/latency/overall_distribution', - signal: abortCtrl.current.signal, - params: { - body: { - ...fetchParams, - percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, - termFilters: [ - { fieldName: EVENT_OUTCOME, fieldValue: EventOutcome.failure }, - ], - }, - }, - }); responseUpdate.errorHistogram = errorHistogram; + responseUpdate.overallHistogram = overallHistogram; + responseUpdate.percentileThresholdValue = percentileThresholdValue; - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -132,7 +136,7 @@ export function useFailedTransactionsCorrelations() { }, }); - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -182,7 +186,7 @@ export function useFailedTransactionsCorrelations() { PROGRESS_STEP_P_VALUES, }); - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } } @@ -204,7 +208,7 @@ export function useFailedTransactionsCorrelations() { setResponse({ ...responseUpdate, loaded: LOADED_DONE, isRunning: false }); setResponse.flush(); } catch (e) { - if (!isCancelledRef.current) { + if (!abortCtrl.current.signal.aborted) { const err = e as Error | IHttpFetchError; setResponse({ error: @@ -219,7 +223,6 @@ export function useFailedTransactionsCorrelations() { }, [fetchParams, setResponse]); const cancelFetch = useCallback(() => { - isCancelledRef.current = true; abortCtrl.current.abort(); setResponse({ isRunning: false, @@ -231,7 +234,6 @@ export function useFailedTransactionsCorrelations() { useEffect(() => { startFetch(); return () => { - isCancelledRef.current = true; abortCtrl.current.abort(); }; }, [startFetch, cancelFetch]); diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts b/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts index 80c465c795b60..358d436f8f0a5 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts +++ b/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts @@ -54,17 +54,11 @@ export function useLatencyCorrelations() { [] ); - // `abortCtrl` is used to cancel individual requests that already started. - // `isCancelledRef` is used to cancel the overall task in between requests in the `startFetch` callback. const abortCtrl = useRef(new AbortController()); - // We're using a ref here because otherwise the startFetch function might have - // a stale value for checking if the task has been cancelled. - const isCancelledRef = useRef(false); const startFetch = useCallback(async () => { abortCtrl.current.abort(); abortCtrl.current = new AbortController(); - isCancelledRef.current = false; setResponse({ ...getInitialResponse(), @@ -100,7 +94,7 @@ export function useLatencyCorrelations() { responseUpdate.overallHistogram = overallHistogram; responseUpdate.percentileThresholdValue = percentileThresholdValue; - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -118,7 +112,7 @@ export function useLatencyCorrelations() { }, }); - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -149,7 +143,7 @@ export function useLatencyCorrelations() { fieldValuePairs.push(...fieldValuePairChunkResponse.fieldValuePairs); } - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -162,7 +156,7 @@ export function useLatencyCorrelations() { }); } - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } @@ -206,7 +200,7 @@ export function useLatencyCorrelations() { PROGRESS_STEP_CORRELATIONS, }); - if (isCancelledRef.current) { + if (abortCtrl.current.signal.aborted) { return; } } @@ -232,7 +226,7 @@ export function useLatencyCorrelations() { }); setResponse.flush(); } catch (e) { - if (!isCancelledRef.current) { + if (!abortCtrl.current.signal.aborted) { const err = e as Error | IHttpFetchError; setResponse({ error: @@ -247,7 +241,6 @@ export function useLatencyCorrelations() { }, [fetchParams, setResponse]); const cancelFetch = useCallback(() => { - isCancelledRef.current = true; abortCtrl.current.abort(); setResponse({ isRunning: false, @@ -259,7 +252,6 @@ export function useLatencyCorrelations() { useEffect(() => { startFetch(); return () => { - isCancelledRef.current = true; abortCtrl.current.abort(); }; }, [startFetch, cancelFetch]);