Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cache icon fro map task #519

Merged
merged 6 commits into from
Jun 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions packages/composites/ui-atoms/src/Icons/MapCacheIcon/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as React from 'react';
import SvgIcon, { SvgIconProps } from '@material-ui/core/SvgIcon';

export const MapCacheIcon = React.forwardRef<SVGSVGElement, SvgIconProps>((props, ref) => {
return (
<SvgIcon {...props} ref={ref} viewBox="0 0 17 17">
<g clipPath="url(#clip0_6712_89419)">
<path
d="M5.68615 2.99228L6.91566 7.15285L4.89438 6.0537C4.31136 7.12586 4.17812 8.3857 4.52399 9.55609C4.86986 10.7265 5.45103 11.4847 6.52318 12.0677C7.19695 12.4341 8.24054 12.5388 8.96464 12.5397L9.41341 14.0583C8.25879 14.1378 6.70623 14.0476 5.68964 13.4944C4.2601 12.717 3.51417 11.5513 3.05301 9.99079C2.59185 8.43027 2.76949 6.75048 3.54686 5.32094L1.52558 4.22179L5.68615 2.99228Z"
fill="#666666"
/>
<path
d="M9.90614 2.76737C11.152 3.02647 12.2756 3.69438 13.0985 4.66504C13.9214 5.6357 14.3964 6.85346 14.4481 8.12494C14.4998 9.39642 14.1252 10.6487 13.3839 11.683C12.6425 12.7173 11.5768 13.4742 10.3561 13.8336L8.74374 8.35689L9.90614 2.76737Z"
fill="#DDDDE5"
/>
</g>
<defs>
<clipPath id="clip0_6712_89419">
<rect width="16" height="16" fill="white" transform="translate(0.406982 0.356445)" />
</clipPath>
</defs>
</SvgIcon>
);
});
1 change: 1 addition & 0 deletions packages/composites/ui-atoms/src/Icons/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export { FlyteLogo } from './FlyteLogo';
export { InfoIcon } from './InfoIcon';
export { RerunIcon } from './RerunIcon';
export { MapCacheIcon } from './MapCacheIcon';
export { MuiLaunchPlanIcon } from './MuiLaunchPlanIcon';
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ export const NodeExecutionDetailsPanelContent: React.FC<NodeExecutionDetailsProp
if (nodeExecution) {
detailsContent = (
<>
<NodeExecutionCacheStatus taskNodeMetadata={nodeExecution.closure.taskNodeMetadata} />
<NodeExecutionCacheStatus execution={nodeExecution} />
<ExecutionTypeDetails details={details} execution={nodeExecution} />
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { timestampToDate } from 'common/utils';
import { CatalogCacheStatus, NodeExecutionPhase } from 'models/Execution/enums';
import { dNode } from 'models/Graph/types';
import { isMapTaskType } from 'models/Task/utils';
import { BarItemData } from './utils';

const WEEK_DURATION_SEC = 7 * 24 * 3600;
Expand All @@ -10,6 +11,7 @@ const EMPTY_BAR_ITEM: BarItemData = {
startOffsetSec: 0,
durationSec: 0,
isFromCache: false,
isMapTaskCache: false,
};

export const getChartDurationData = (
Expand All @@ -20,7 +22,8 @@ export const getChartDurationData = (

let totalDurationSec = 0;
const initialStartTime = startedAt.getTime();
const result: BarItemData[] = nodes.map(({ execution }) => {

const result: BarItemData[] = nodes.map(({ execution, value }) => {
if (!execution) {
return EMPTY_BAR_ITEM;
}
Expand All @@ -29,6 +32,9 @@ export const getChartDurationData = (
const isFromCache =
execution.closure.taskNodeMetadata?.cacheStatus === CatalogCacheStatus.CACHE_HIT;

const isMapTaskCache =
isMapTaskType(value?.template?.type) && value?.template?.metadata?.cacheSerializable;

// Offset values
let startOffset = 0;
const startedAt = execution.closure.startedAt;
Expand Down Expand Up @@ -67,7 +73,7 @@ export const getChartDurationData = (

const startOffsetSec = Math.trunc(startOffset / 1000);
totalDurationSec = Math.max(totalDurationSec, startOffsetSec + durationSec);
return { phase, startOffsetSec, durationSec, isFromCache };
return { phase, startOffsetSec, durationSec, isFromCache, isMapTaskCache };
});

// Do we want to get initialStartTime from different place, to avoid recalculating it.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { getNodeExecutionPhaseConstants } from 'components/Executions/utils';
import { primaryTextColor } from 'components/Theme/constants';
import { NodeExecutionPhase } from 'models/Execution/enums';
import t from 'components/Executions/strings';

export const CASHED_GREEN = 'rgba(74,227,174,0.25)'; // statusColors.SUCCESS (Mint20) with 25% opacity
export const TRANSPARENT = 'rgba(0, 0, 0, 0)';
Expand All @@ -10,6 +11,7 @@ export interface BarItemData {
startOffsetSec: number;
durationSec: number;
isFromCache: boolean;
isMapTaskCache: boolean;
}

interface ChartDataInput {
Expand Down Expand Up @@ -60,11 +62,24 @@ export const generateChartData = (data: BarItemData[]): ChartDataInput => {
// don't show Label if there is now duration yet.
const labelString = element.durationSec > 0 ? durationString : '';

const generateTooltipLabelText = (element: BarItemData): string[] => {
if (element.isMapTaskCache) return [tooltipString, t('mapCacheMessage')];
if (element.isFromCache) return [tooltipString, t('readFromCache')];

return [tooltipString];
};

const generateBarLabelText = (element: BarItemData): string => {
if (element.isMapTaskCache) return '\u229A ' + t('mapCacheMessage');
if (element.isFromCache) return '\u229A ' + t('fromCache');
return labelString;
};

durations.push(element.durationSec);
startOffset.push(element.startOffsetSec);
offsetColor.push(element.isFromCache ? CASHED_GREEN : TRANSPARENT);
tooltipLabel.push(element.isFromCache ? [tooltipString, 'Read from cache'] : [tooltipString]);
barLabel.push(element.isFromCache ? '\u229A From cache' : labelString);
tooltipLabel.push(generateTooltipLabelText(element));
barLabel.push(generateBarLabelText(element));
barColor.push(phaseConstant.badgeColor);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,21 @@ describe('ExecutionDetails > Timeline > BarChart', () => {

it('generateChartData properly generates map of data for ChartBars', () => {
const chartData = generateChartData(mockbarItems);
expect(chartData.durations).toEqual([15, 11, 23, 0]);
expect(chartData.startOffset).toEqual([0, 5, 17, 39]);
expect(chartData.offsetColor).toEqual([TRANSPARENT, CASHED_GREEN, TRANSPARENT, TRANSPARENT]);
expect(chartData.durations).toEqual([15, 11, 23, 0, 11]);
expect(chartData.startOffset).toEqual([0, 5, 17, 39, 5]);
expect(chartData.offsetColor).toEqual([
TRANSPARENT,
CASHED_GREEN,
TRANSPARENT,
TRANSPARENT,
TRANSPARENT,
]);
// labels looks as expected
expect(chartData.barLabel[0]).toEqual(formatSecondsToHmsFormat(mockbarItems[0].durationSec));
expect(chartData.barLabel[1]).toEqual('\u229A From cache');
expect(chartData.barLabel[3]).toEqual('');
expect(chartData.barLabel[4]).toEqual(
"\u229A Check the detail panel for each task's cache status.",
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,41 @@ const getMockNodeExecution = (
};

export const mockbarItems = [
{ phase: NodeExecutionPhase.FAILED, startOffsetSec: 0, durationSec: 15, isFromCache: false },
{ phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 5, durationSec: 11, isFromCache: true },
{ phase: NodeExecutionPhase.RUNNING, startOffsetSec: 17, durationSec: 23, isFromCache: false },
{ phase: NodeExecutionPhase.QUEUED, startOffsetSec: 39, durationSec: 0, isFromCache: false },
{
phase: NodeExecutionPhase.FAILED,
startOffsetSec: 0,
durationSec: 15,
isFromCache: false,
isMapTaskCache: false,
},
{
phase: NodeExecutionPhase.SUCCEEDED,
startOffsetSec: 5,
durationSec: 11,
isFromCache: true,
isMapTaskCache: false,
},
{
phase: NodeExecutionPhase.RUNNING,
startOffsetSec: 17,
durationSec: 23,
isFromCache: false,
isMapTaskCache: false,
},
{
phase: NodeExecutionPhase.QUEUED,
startOffsetSec: 39,
durationSec: 0,
isFromCache: false,
isMapTaskCache: false,
},
{
phase: NodeExecutionPhase.SUCCEEDED,
startOffsetSec: 5,
durationSec: 11,
isFromCache: false,
isMapTaskCache: true,
},
];

export const getMockExecutionsForBarChart = (startTimeSec: number) => {
Expand All @@ -73,5 +104,6 @@ export const getMockExecutionsForBarChart = (startTimeSec: number) => {
getMockNodeExecution(start, NodeExecutionPhase.SUCCEEDED, 5, 11, CatalogCacheStatus.CACHE_HIT),
getMockNodeExecution(start, NodeExecutionPhase.RUNNING, 17, 23, CatalogCacheStatus.CACHE_MISS),
getMockNodeExecution(start, NodeExecutionPhase.QUEUED, 39, 0),
getMockNodeExecution(start, NodeExecutionPhase.SUCCEEDED, 5, 11, CatalogCacheStatus.MAP_CACHE),
];
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@ import classnames from 'classnames';
import { assertNever } from 'common/utils';
import { PublishedWithChangesOutlined } from 'components/common/PublishedWithChanges';
import { useCommonStyles } from 'components/common/styles';
import { NodeExecutionDetails } from 'components/Executions/types';
import { useNodeExecutionContext } from 'components/Executions/contextProvider/NodeExecutionDetails';
import { CatalogCacheStatus } from 'models/Execution/enums';
import { TaskNodeMetadata } from 'models/Execution/types';
import { NodeExecution, TaskExecutionIdentifier } from 'models/Execution/types';
import { MapCacheIcon } from '@flyteconsole/ui-atoms';
import * as React from 'react';
import { isMapTaskType } from 'models/Task/utils';
import { Link as RouterLink } from 'react-router-dom';
import { Routes } from 'routes/routes';
import {
Expand Down Expand Up @@ -50,6 +54,9 @@ export const NodeExecutionCacheStatusIcon: React.FC<
case CatalogCacheStatus.CACHE_PUT_FAILURE: {
return <ErrorOutlined {...props} ref={ref} />;
}
case CatalogCacheStatus.MAP_CACHE: {
return <MapCacheIcon {...props} ref={ref} />;
}
default: {
assertNever(status);
return null;
Expand All @@ -58,7 +65,7 @@ export const NodeExecutionCacheStatusIcon: React.FC<
});

export interface NodeExecutionCacheStatusProps {
taskNodeMetadata?: TaskNodeMetadata;
execution: NodeExecution;
/** `normal` will render an icon with description message beside it
* `iconOnly` will render just the icon with the description as a tooltip
*/
Expand All @@ -68,20 +75,78 @@ export interface NodeExecutionCacheStatusProps {
* the cache status with a descriptive message. For `Core.CacheCatalogStatus.CACHE_HIT`,
* it will also attempt to render a link to the source `WorkflowExecution` (normal
* variant only).
*
* For Map Tasks, we will check the NodeExecutionDetail for the cache status instead. Since map tasks
* cotains multiple tasks, the logic of the cache status is different.
*/
export const NodeExecutionCacheStatus: React.FC<NodeExecutionCacheStatusProps> = ({
taskNodeMetadata,
execution,
variant = 'normal',
}) => {
const taskNodeMetadata = execution.closure?.taskNodeMetadata;
const detailsContext = useNodeExecutionContext();
const [nodeDetails, setNodeDetails] = React.useState<NodeExecutionDetails | undefined>();

React.useEffect(() => {
let isCurrent = true;
detailsContext.getNodeExecutionDetails(execution).then((res) => {
if (isCurrent) {
setNodeDetails(res);
}
});
return () => {
isCurrent = false;
};
});

if (isMapTaskType(nodeDetails?.taskTemplate?.type)) {
if (nodeDetails?.taskTemplate?.metadata?.cacheSerializable) {
return <CacheStatus cacheStatus={CatalogCacheStatus.MAP_CACHE} variant={variant} />;
}
}

// cachestatus can be 0
if (taskNodeMetadata?.cacheStatus == null) {
return null;
}

const sourceTaskExecution = taskNodeMetadata.catalogKey?.sourceTaskExecution;

return (
<CacheStatus
cacheStatus={taskNodeMetadata.cacheStatus}
sourceTaskExecution={sourceTaskExecution}
variant={variant}
/>
);
};

export interface CacheStatusProps {
cacheStatus: CatalogCacheStatus | null | undefined;
/** `normal` will render an icon with description message beside it
* `iconOnly` will render just the icon with the description as a tooltip
*/
variant?: 'normal' | 'iconOnly';
sourceTaskExecution?: TaskExecutionIdentifier;
iconStyles?: React.CSSProperties;
}

export const CacheStatus: React.FC<CacheStatusProps> = ({
cacheStatus,
sourceTaskExecution,
variant = 'normal',
iconStyles,
}) => {
const commonStyles = useCommonStyles();
const styles = useStyles();
if (taskNodeMetadata == null || taskNodeMetadata.cacheStatus == null) {

if (cacheStatus == null) {
return null;
}

const message = cacheStatusMessages[taskNodeMetadata.cacheStatus] || unknownCacheStatusString;
const message = cacheStatusMessages[cacheStatus] || unknownCacheStatusString;

const sourceExecutionId = taskNodeMetadata.catalogKey?.sourceTaskExecution;
const sourceExecutionId = sourceTaskExecution;
const sourceExecutionLink = sourceExecutionId ? (
<RouterLink
className={classnames(commonStyles.primaryLink, styles.sourceExecutionLink)}
Expand All @@ -95,18 +160,19 @@ export const NodeExecutionCacheStatus: React.FC<NodeExecutionCacheStatusProps> =
<Tooltip title={message} aria-label="cache status">
<NodeExecutionCacheStatusIcon
className={classnames(
commonStyles.iconLeft,
// commonStyles.iconLeft,
anrusina marked this conversation as resolved.
Show resolved Hide resolved
commonStyles.iconRight,
commonStyles.iconSecondary,
)}
status={taskNodeMetadata.cacheStatus}
style={iconStyles}
status={cacheStatus}
/>
</Tooltip>
) : (
<div>
<Typography className={styles.cacheStatus} variant="subtitle1" color="textSecondary">
<NodeExecutionCacheStatusIcon
status={taskNodeMetadata.cacheStatus}
status={cacheStatus}
className={classnames(commonStyles.iconSecondary, commonStyles.iconLeft)}
/>
{message}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,13 @@ export function generateColumns(
label: 'type',
},
{
cellRenderer: ({
execution: {
closure: { phase = NodeExecutionPhase.UNDEFINED, taskNodeMetadata },
},
}) => (
cellRenderer: ({ execution }) => (
<>
<ExecutionStatusBadge phase={phase} type="node" />
{hasCacheStatus(taskNodeMetadata) ? (
<NodeExecutionCacheStatus taskNodeMetadata={taskNodeMetadata} variant="iconOnly" />
) : null}
<ExecutionStatusBadge
phase={execution.closure?.phase ?? NodeExecutionPhase.UNDEFINED}
type="node"
/>
<NodeExecutionCacheStatus execution={execution} variant="iconOnly" />
</>
),
className: styles.columnStatus,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ describe('NodeExecutionsTable', () => {
CatalogCacheStatus.CACHE_LOOKUP_FAILURE,
CatalogCacheStatus.CACHE_POPULATED,
CatalogCacheStatus.CACHE_PUT_FAILURE,
CatalogCacheStatus.CACHE_MISS,
CatalogCacheStatus.CACHE_DISABLED,
].forEach((cacheStatusValue) =>
it(`renders correct icon for ${CatalogCacheStatus[cacheStatusValue]}`, async () => {
taskNodeMetadata.cacheStatus = cacheStatusValue;
Expand All @@ -254,19 +256,6 @@ describe('NodeExecutionsTable', () => {
);
}),
);

[CatalogCacheStatus.CACHE_DISABLED, CatalogCacheStatus.CACHE_MISS].forEach(
(cacheStatusValue) =>
it(`renders no icon for ${CatalogCacheStatus[cacheStatusValue]}`, async () => {
taskNodeMetadata.cacheStatus = cacheStatusValue;
updateNodeExecutions([cachedNodeExecution]);
const { getByText, queryByTitle } = renderTable();
await waitFor(() => {
getByText(cachedNodeExecution.id.nodeId);
});
expect(queryByTitle(cacheStatusMessages[cacheStatusValue])).toBeNull();
}),
);
});
});

Expand Down
Loading