From 8ca177faa4fa0dab7779da833531a003815750bd Mon Sep 17 00:00:00 2001 From: Marco polo Date: Tue, 11 Jun 2024 04:34:25 -1000 Subject: [PATCH] [perf] Overview pages - don't show loading state when workspace cache available (#22456) ## Summary & Motivation Lost some loading state stuff while rebasing these changes... adds back skipping the loading state if the workspace cache has data for us to use in it. Also made WorkspaceSensors/WorkspaceSchedules pages use the workspace cache if available. ## How I Tested These Changes Load OverviewJobs/Sensors/Schedules/Resources for a large customer and see these pages load immediately. --- .../ui-core/src/jobs/JobsPageContent.tsx | 10 ++++--- .../src/overview/OverviewResourcesRoot.tsx | 2 +- .../src/overview/OverviewSchedules.tsx | 8 +++-- .../ui-core/src/overview/OverviewSensors.tsx | 9 +++--- .../src/workspace/WorkspaceJobsRoot.tsx | 29 ++++++++++++------- .../src/workspace/WorkspaceSchedulesRoot.tsx | 25 ++++++++++++++-- .../src/workspace/WorkspaceSensorsRoot.tsx | 25 ++++++++++++++-- 7 files changed, 79 insertions(+), 29 deletions(-) diff --git a/js_modules/dagster-ui/packages/ui-core/src/jobs/JobsPageContent.tsx b/js_modules/dagster-ui/packages/ui-core/src/jobs/JobsPageContent.tsx index 6c103a0504cb9..be4f15ceca3bc 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/jobs/JobsPageContent.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/jobs/JobsPageContent.tsx @@ -51,7 +51,7 @@ export const JobsPageContent = () => { notifyOnNetworkStatusChange: true, }, ); - const {data, loading} = queryResultOverview; + const {data, loading: queryLoading} = queryResultOverview; const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS); @@ -72,7 +72,9 @@ export const JobsPageContent = () => { ); }, [cachedData, data, visibleRepos]); - useBlockTraceUntilTrue('OverviewJobs', !!data || !workspaceLoading); + const loading = !data && workspaceLoading; + + useBlockTraceUntilTrue('OverviewJobs', !loading); const sanitizedSearch = searchValue.trim().toLocaleLowerCase(); const anySearch = sanitizedSearch.length > 0; @@ -88,7 +90,7 @@ export const JobsPageContent = () => { }, [repoBuckets, sanitizedSearch]); const content = () => { - if (loading && !data) { + if (loading) { return ( @@ -143,7 +145,7 @@ export const JobsPageContent = () => { return ; }; - const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data); + const showSearchSpinner = queryLoading && !data; return ( <> diff --git a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewResourcesRoot.tsx b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewResourcesRoot.tsx index 699a509392c83..bf35b25be89cb 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewResourcesRoot.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewResourcesRoot.tsx @@ -143,7 +143,7 @@ export const OverviewResourcesRoot = () => { return ; }; - const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data); + const showSearchSpinner = queryLoading && !data; return ( diff --git a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSchedules.tsx b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSchedules.tsx index 0f3385cf58fba..efaf6343d2fea 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSchedules.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSchedules.tsx @@ -61,7 +61,7 @@ export const OverviewSchedules = () => { notifyOnNetworkStatusChange: true, }, ); - const {data, loading} = queryResultOverview; + const {data, loading: queryLoading} = queryResultOverview; useBlockTraceOnQueryResult(queryResultOverview, 'OverviewSchedulesQuery'); const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS); @@ -166,8 +166,10 @@ export const OverviewSchedules = () => { const viewerHasAnyInstigationPermission = allPermissionedScheduleKeys.length > 0; const checkedCount = checkedSchedules.length; + const loading = workspaceLoading && !repoCount && queryLoading && !data; + const content = () => { - if (loading && !data) { + if (loading) { return ( @@ -238,7 +240,7 @@ export const OverviewSchedules = () => { ); }; - const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data); + const showSearchSpinner = queryLoading && !data; return ( <> diff --git a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx index af81243c48868..5a70e8a2a5eb8 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx @@ -103,7 +103,7 @@ export const OverviewSensors = () => { notifyOnNetworkStatusChange: true, }, ); - const {data, loading} = queryResultOverview; + const {data, loading: queryLoading} = queryResultOverview; useBlockTraceOnQueryResult(queryResultOverview, 'OverviewSensorsQuery'); @@ -215,8 +215,9 @@ export const OverviewSensors = () => { const viewerHasAnyInstigationPermission = allPermissionedSensorKeys.length > 0; const checkedCount = checkedSensors.length; + const loading = workspaceLoading && queryLoading && !data; const content = () => { - if (loading && !data) { + if (loading) { return ( @@ -287,7 +288,7 @@ export const OverviewSensors = () => { ); }; - const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data); + const showSearchSpinner = queryLoading && !data; return ( <> @@ -337,7 +338,7 @@ export const OverviewSensors = () => { {activeFiltersJsx} ) : null} - {loading && !repoCount ? ( + {loading ? ( diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceJobsRoot.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceJobsRoot.tsx index 6dd504dd3e677..b19936133bf16 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceJobsRoot.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceJobsRoot.tsx @@ -17,6 +17,7 @@ import {useDocumentTitle} from '../hooks/useDocumentTitle'; import {useQueryPersistedState} from '../hooks/useQueryPersistedState'; import {usePageLoadTrace} from '../performance'; import {useBlockTraceUntilTrue} from '../performance/TraceContext'; +import {SearchInputSpinner} from '../ui/SearchInputSpinner'; const NO_REPOS_EMPTY_ARR: any[] = []; @@ -43,13 +44,7 @@ export const WorkspaceJobsRoot = ({repoAddress}: {repoAddress: RepoAddress}) => variables: {selector}, }, ); - const {data, loading} = queryResultOverview; - - useLayoutEffect(() => { - if (!loading) { - trace.endTrace(); - } - }, [loading, trace]); + const {data, loading: queryLoading} = queryResultOverview; const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS); @@ -57,16 +52,23 @@ export const WorkspaceJobsRoot = ({repoAddress}: {repoAddress: RepoAddress}) => const anySearch = sanitizedSearch.length > 0; const jobs = useMemo(() => { - if (repo) { - return repo.repository.pipelines; - } if (data?.repositoryOrError.__typename === 'Repository') { return data.repositoryOrError.pipelines; } + if (repo) { + return repo.repository.pipelines; + } return NO_REPOS_EMPTY_ARR; }, [data, repo]); - useBlockTraceUntilTrue('WorkspaceJobs', jobs !== NO_REPOS_EMPTY_ARR); + const loading = jobs === NO_REPOS_EMPTY_ARR; + + useLayoutEffect(() => { + if (!loading) { + trace.endTrace(); + } + }, [loading, trace]); + useBlockTraceUntilTrue('WorkspaceJobs', !loading); const filteredBySearch = useMemo(() => { const searchToLower = sanitizedSearch.toLocaleLowerCase(); @@ -118,6 +120,8 @@ export const WorkspaceJobsRoot = ({repoAddress}: {repoAddress: RepoAddress}) => return ; }; + const showSearchSpinner = !data && queryLoading; + return ( onChange={(e) => setSearchValue(e.target.value)} placeholder="Filter by job name…" style={{width: '340px'}} + rightElement={ + showSearchSpinner ? : undefined + } /> {loading && !data ? ( diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSchedulesRoot.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSchedulesRoot.tsx index 05e24d87fd4c2..31271a9d6bd92 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSchedulesRoot.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSchedulesRoot.tsx @@ -3,6 +3,7 @@ import {Box, Colors, NonIdealState, Spinner, TextInput, Tooltip} from '@dagster- import {useMemo} from 'react'; import {VirtualizedScheduleTable} from './VirtualizedScheduleTable'; +import {useRepository} from './WorkspaceContext'; import {WorkspaceHeader} from './WorkspaceHeader'; import {repoAddressAsHumanString} from './repoAddressAsString'; import {repoAddressToSelector} from './repoAddressToSelector'; @@ -25,10 +26,16 @@ import {makeScheduleKey} from '../schedules/makeScheduleKey'; import {CheckAllBox} from '../ui/CheckAllBox'; import {useFilters} from '../ui/Filters'; import {useInstigationStatusFilter} from '../ui/Filters/useInstigationStatusFilter'; +import {SearchInputSpinner} from '../ui/SearchInputSpinner'; + +// Reuse this reference to distinguish no sensors case from data is still loading case; +const NO_DATA_EMPTY_ARR: any[] = []; export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress}) => { useTrackPageView(); + const repo = useRepository(repoAddress); + const repoName = repoAddressAsHumanString(repoAddress); useDocumentTitle(`Schedules: ${repoName}`); @@ -51,7 +58,7 @@ export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress} }, ); useBlockTraceOnQueryResult(queryResultOverview, 'WorkspaceSchedulesQuery'); - const {data, loading} = queryResultOverview; + const {data, loading: queryLoading} = queryResultOverview; const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS); const sanitizedSearch = searchValue.trim().toLocaleLowerCase(); @@ -61,8 +68,13 @@ export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress} if (data?.repositoryOrError.__typename === 'Repository') { return data.repositoryOrError.schedules; } - return []; - }, [data]); + if (repo) { + return repo.repository.schedules; + } + return NO_DATA_EMPTY_ARR; + }, [data, repo]); + + const loading = NO_DATA_EMPTY_ARR === schedules; const {state: runningState} = runningStateFilter; const filteredByRunningState = useMemo(() => { @@ -165,6 +177,8 @@ export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress} ); }; + const showSearchSpinner = queryLoading && !data; + return ( + ) : undefined + } /> { useTrackPageView(); + const repo = useRepository(repoAddress); + const repoName = repoAddressAsHumanString(repoAddress); useDocumentTitle(`Sensors: ${repoName}`); @@ -51,7 +58,7 @@ export const WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress}) }, ); useBlockTraceOnQueryResult(queryResultOverview, 'WorkspaceSensorsQuery'); - const {data, loading} = queryResultOverview; + const {data, loading: queryLoading} = queryResultOverview; const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS); const sanitizedSearch = searchValue.trim().toLocaleLowerCase(); @@ -61,8 +68,13 @@ export const WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress}) if (data?.repositoryOrError.__typename === 'Repository') { return data.repositoryOrError.sensors; } - return []; - }, [data]); + if (repo) { + return repo.repository.sensors; + } + return NO_DATA_EMPTY_ARR; + }, [repo, data]); + + const loading = NO_DATA_EMPTY_ARR === sensors; const {state: runningState} = runningStateFilter; const filteredByRunningState = useMemo(() => { @@ -165,6 +177,8 @@ export const WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress}) ); }; + const showSearchSpinner = queryLoading && !data; + return ( setSearchValue(e.target.value)} placeholder="Filter by sensor name…" style={{width: '340px'}} + rightElement={ + showSearchSpinner ? ( + + ) : undefined + } />