[ML] Use abort signal instead of isCancelledRef.

elastic · Nov 5, 2021 · 97890cf · 97890cf
1 parent 3e411cd
commit 97890cf
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 54 deletions.
diff --git a/...gins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx b/...gins/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/...ck/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts b/...ck/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<ResponseErrorBody>;
         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
@@ -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<ResponseErrorBody>;
         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]);