Skip to content

Commit

Permalink
[perf] Overview pages - don't show loading state when workspace cache…
Browse files Browse the repository at this point in the history 
… 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.
  • Loading branch information
@salazarm
salazarm authored Jun 11, 2024
1 parent 8fe0426 commit 8ca177f
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 29 deletions.
10 changes: 6 additions & 4 deletions js_modules/dagster-ui/packages/ui-core/src/jobs/JobsPageContent.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const JobsPageContent = () => {
notifyOnNetworkStatusChange: true,
},
);
const {data, loading} = queryResultOverview;
const {data, loading: queryLoading} = queryResultOverview;

const refreshState = useQueryRefreshAtInterval(queryResultOverview, FIFTEEN_SECONDS);

Expand All @@ -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;
Expand All @@ -88,7 +90,7 @@ export const JobsPageContent = () => {
}, [repoBuckets, sanitizedSearch]);

const content = () => {
if (loading && !data) {
if (loading) {
return (
<Box flex={{direction: 'row', justifyContent: 'center'}} style={{paddingTop: '100px'}}>
<Box flex={{direction: 'row', alignItems: 'center', gap: 16}}>
Expand Down Expand Up @@ -143,7 +145,7 @@ export const JobsPageContent = () => {
return <OverviewJobsTable repos={filteredBySearch} />;
};

const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data);
const showSearchSpinner = queryLoading && !data;

return (
<>
Expand Down
2 changes: 1 addition & 1 deletion js_modules/dagster-ui/packages/ui-core/src/overview/OverviewResourcesRoot.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export const OverviewResourcesRoot = () => {
return <OverviewResourcesTable repos={filteredBySearch} />;
};

const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data);
const showSearchSpinner = queryLoading && !data;

return (
<Box flex={{direction: 'column'}} style={{height: '100%', overflow: 'hidden'}}>
Expand Down
8 changes: 5 additions & 3 deletions js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSchedules.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 (
<Box flex={{direction: 'row', justifyContent: 'center'}} style={{paddingTop: '100px'}}>
<Box flex={{direction: 'row', alignItems: 'center', gap: 16}}>
Expand Down Expand Up @@ -238,7 +240,7 @@ export const OverviewSchedules = () => {
);
};

const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data);
const showSearchSpinner = queryLoading && !data;

return (
<>
Expand Down
9 changes: 5 additions & 4 deletions js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const OverviewSensors = () => {
notifyOnNetworkStatusChange: true,
},
);
const {data, loading} = queryResultOverview;
const {data, loading: queryLoading} = queryResultOverview;

useBlockTraceOnQueryResult(queryResultOverview, 'OverviewSensorsQuery');

Expand Down Expand Up @@ -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 (
<Box flex={{direction: 'row', justifyContent: 'center'}} style={{paddingTop: '100px'}}>
<Box flex={{direction: 'row', alignItems: 'center', gap: 16}}>
Expand Down Expand Up @@ -287,7 +288,7 @@ export const OverviewSensors = () => {
);
};

const showSearchSpinner = (workspaceLoading && !repoCount) || (loading && !data);
const showSearchSpinner = queryLoading && !data;

return (
<>
Expand Down Expand Up @@ -337,7 +338,7 @@ export const OverviewSensors = () => {
{activeFiltersJsx}
</Box>
) : null}
{loading && !repoCount ? (
{loading ? (
<Box padding={64}>
<Spinner purpose="page" />
</Box>
Expand Down
29 changes: 18 additions & 11 deletions js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceJobsRoot.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];

Expand All @@ -43,30 +44,31 @@ 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);

const sanitizedSearch = searchValue.trim().toLocaleLowerCase();
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();
Expand Down Expand Up @@ -118,6 +120,8 @@ export const WorkspaceJobsRoot = ({repoAddress}: {repoAddress: RepoAddress}) =>
return <VirtualizedJobTable repoAddress={repoAddress} jobs={filteredBySearch} />;
};

const showSearchSpinner = !data && queryLoading;

return (
<Box flex={{direction: 'column'}} style={{height: '100%', overflow: 'hidden'}}>
<WorkspaceHeader
Expand All @@ -133,6 +137,9 @@ export const WorkspaceJobsRoot = ({repoAddress}: {repoAddress: RepoAddress}) =>
onChange={(e) => setSearchValue(e.target.value)}
placeholder="Filter by job name…"
style={{width: '340px'}}
rightElement={
showSearchSpinner ? <SearchInputSpinner tooltipContent="Loading jobs…" /> : undefined
}
/>
</Box>
{loading && !data ? (
Expand Down
25 changes: 22 additions & 3 deletions js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSchedulesRoot.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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}`);

Expand All @@ -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();
Expand All @@ -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(() => {
Expand Down Expand Up @@ -165,6 +177,8 @@ export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress}
);
};

const showSearchSpinner = queryLoading && !data;

return (
<Box flex={{direction: 'column'}} style={{height: '100%', overflow: 'hidden'}}>
<WorkspaceHeader
Expand All @@ -185,6 +199,11 @@ export const WorkspaceSchedulesRoot = ({repoAddress}: {repoAddress: RepoAddress}
}}
placeholder="Filter by schedule name…"
style={{width: '340px'}}
rightElement={
showSearchSpinner ? (
<SearchInputSpinner tooltipContent="Loading schedules…" />
) : undefined
}
/>
</Box>
<Tooltip
Expand Down
25 changes: 22 additions & 3 deletions js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceSensorsRoot.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {Box, Colors, NonIdealState, Spinner, TextInput, Tooltip} from '@dagster-
import {useMemo} from 'react';

import {VirtualizedSensorTable} from './VirtualizedSensorTable';
import {useRepository} from './WorkspaceContext';
import {WorkspaceHeader} from './WorkspaceHeader';
import {repoAddressAsHumanString} from './repoAddressAsString';
import {repoAddressToSelector} from './repoAddressToSelector';
Expand All @@ -25,10 +26,16 @@ import {makeSensorKey} from '../sensors/makeSensorKey';
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 WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress}) => {
useTrackPageView();

const repo = useRepository(repoAddress);

const repoName = repoAddressAsHumanString(repoAddress);
useDocumentTitle(`Sensors: ${repoName}`);

Expand All @@ -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();
Expand All @@ -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(() => {
Expand Down Expand Up @@ -165,6 +177,8 @@ export const WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress})
);
};

const showSearchSpinner = queryLoading && !data;

return (
<Box flex={{direction: 'column'}} style={{height: '100%', overflow: 'hidden'}}>
<WorkspaceHeader
Expand All @@ -182,6 +196,11 @@ export const WorkspaceSensorsRoot = ({repoAddress}: {repoAddress: RepoAddress})
onChange={(e) => setSearchValue(e.target.value)}
placeholder="Filter by sensor name…"
style={{width: '340px'}}
rightElement={
showSearchSpinner ? (
<SearchInputSpinner tooltipContent="Loading sensors…" />
) : undefined
}
/>
</Box>
<Tooltip
Expand Down

1 comment on commit 8ca177f

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-ip8d4y6rv-elementl.vercel.app

Built with commit 8ca177f.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.