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.

Sign up for GitHub

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

Generate charts for Fio benchmark #3549

Merged
merged 5 commits into from
Oct 3, 2023
Merged

Generate charts for Fio benchmark #3549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a5bf04b
PBENCH-1214
MVarshini Aug 8, 2023
02900f8
fio key change
MVarshini Sep 6, 2023
ce6319d
quisby version update
MVarshini Sep 7, 2023
02a66e2
review comments
MVarshini Sep 26, 2023
381778c
null the array
MVarshini Sep 26, 2023
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
114 changes: 82 additions & 32 deletions dashboard/src/actions/comparisonActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import * as CONSTANTS from "assets/constants/compareConstants";
import * as TYPES from "./types.js";

import { DANGER, ERROR_MSG } from "assets/constants/toastConstants";
import { DANGER, ERROR_MSG, WARNING } from "assets/constants/toastConstants";

import API from "../utils/axiosInstance";
import { showToast } from "./toastActions";
import { uriTemplate } from "../utils/helper";

const chartTitleMap = {
const uperfChartTitleMap = {
gb_sec: "Bandwidth",
trans_sec: "Transactions/second",
usec: "Latency",
Expand Down Expand Up @@ -34,12 +35,12 @@ export const getQuisbyData = (dataset) => async (dispatch, getState) => {
type: TYPES.IS_UNSUPPORTED_TYPE,
payload: "",
});
dispatch(parseChartData());
dispatch(parseChartData(response.data.benchmark));
}
} catch (error) {
const isCompareSwitchChecked = getState().comparison.isCompareSwitchChecked;
if (
error?.response?.data &&
error.response.data?.message
error?.response?.data?.message
?.toLowerCase()
.includes("unsupported benchmark")
) {
Expand All @@ -50,18 +51,67 @@ export const getQuisbyData = (dataset) => async (dispatch, getState) => {
} else {
dispatch(showToast(DANGER, ERROR_MSG));
}
dispatch({
type: TYPES.SET_QUISBY_DATA,
payload: [],
});
const type = isCompareSwitchChecked
? TYPES.SET_COMPARE_DATA
: TYPES.SET_PARSED_DATA;

dispatch({
type,
payload: [],
});
dispatch({ type: TYPES.NETWORK_ERROR });
}
dispatch({ type: TYPES.COMPLETED });
};
const COLORS = ["#8BC1F7", "#0066CC", "#519DE9", "#004B95", "#002F5D"];
export const parseChartData = () => (dispatch, getState) => {
const COLORS = [
CONSTANTS.COLOR1,
CONSTANTS.COLOR2,
CONSTANTS.COLOR3,
CONSTANTS.COLOR4,
CONSTANTS.COLOR5,
];

const getChartValues = (run, benchmarkType) => {
const benchmark = benchmarkType.toLowerCase();
const chartTitle = {
uperf: `Uperf: ${uperfChartTitleMap[run.metrics_unit.toLowerCase()]} | ${
run.test_name
}`,
fio: `Fio: ${run.test_name} | ${run.metrics_unit.toLowerCase()}`,
};
const yaxisTitle = {
uperf: run.metrics_unit,
fio: "Mb/sec",
};
const keys = {
uperf: "name",
fio: "iteration_name",
};
const values = {
uperf: "time_taken",
fio: "value",
};
const obj = {
chartTitle: chartTitle[benchmark],
yAxis: yaxisTitle[benchmark],
keyToParse: keys[benchmark],
valueToGet: values[benchmark],
};

return obj;
};
export const parseChartData = (benchmark) => (dispatch, getState) => {
const response = getState().comparison.data.data;
const isCompareSwitchChecked = getState().comparison.isCompareSwitchChecked;
const chartData = [];
let i = 0;

for (const run of response) {
const chartObj = getChartValues(run, benchmark);
const options = {
responsive: true,
maintainAspectRatio: false,
Expand All @@ -74,12 +124,9 @@ export const parseChartData = () => (dispatch, getState) => {
display: true,
position: "bottom",
},

title: {
display: true,
text: `Uperf: ${chartTitleMap[run.metrics_unit.toLowerCase()]} | ${
run.test_name
}`,
text: chartObj.chartTitle,
},
},
scales: {
Expand All @@ -92,15 +139,15 @@ export const parseChartData = () => (dispatch, getState) => {
y: {
title: {
display: true,
text: run.metrics_unit,
text: chartObj.yAxis,
},
},
},
};

const datasets = [];
const data = {
labels: [...new Set(run.instances.map((i) => i.name))],
labels: [...new Set(run.instances.map((i) => i[chartObj.keyToParse]))],
id: `${run.test_name}_${run.metrics_unit}`,
datasets,
};
Expand All @@ -111,11 +158,9 @@ export const parseChartData = () => (dispatch, getState) => {
}, Object.create(null));

for (const [key, value] of Object.entries(result)) {
console.log(key);

const map = {};
Comment on lines 158 to 161
Copy link
Member

Choose a reason for hiding this comment

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

Noting (new) line 158, should we be using Object.create(null) at line 161 instead of {}?

for (const element of value) {
map[element.name] = element.time_taken.trim();
map[element[chartObj.keyToParse]] = element[chartObj.valueToGet].trim();
}
webbnh marked this conversation as resolved.
Show resolved Hide resolved
const mappedData = data.labels.map((label) => {
return map[label];
Expand All @@ -139,32 +184,37 @@ export const parseChartData = () => (dispatch, getState) => {
});
};

export const toggleCompareSwitch = () => (dispatch, getState) => {
dispatch({
type: TYPES.TOGGLE_COMPARE_SWITCH,
payload: !getState().comparison.isCompareSwitchChecked,
});
};
export const toggleCompareSwitch = () => ({
type: TYPES.TOGGLE_COMPARE_SWITCH,
});
Comment on lines +187 to +189
Copy link
Member

Choose a reason for hiding this comment

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

If you omit the trailing comma on line 188, I think this will all fit on one line.


export const setSelectedId = (isChecked, rId) => (dispatch, getState) => {
let selectedIds = [...getState().comparison.selectedResourceIds];
if (isChecked) {
selectedIds = [...selectedIds, rId];
const prev = getState().comparison.selectedResourceIds;
const selectedIds = isChecked
? [...prev, rId]
: prev.filter((id) => id !== rId);

if (selectedIds.length > CONSTANTS.MAX_DATASETS_COMPARE) {
dispatch(
showToast(
WARNING,
`Not more than ${CONSTANTS.MAX_DATASETS_COMPARE} datasets can be compared`
)
);
} else {
selectedIds = selectedIds.filter((item) => item !== rId);
dispatch({
type: TYPES.SET_SELECTED_RESOURCE_ID,
payload: selectedIds,
});
}
dispatch({
type: TYPES.SET_SELECTED_RESOURCE_ID,
payload: selectedIds,
});
};

export const compareMultipleDatasets = () => async (dispatch, getState) => {
try {
dispatch({ type: TYPES.LOADING });

const endpoints = getState().apiEndpoint.endpoints;
const selectedIds = [...getState().comparison.selectedResourceIds];
const selectedIds = getState().comparison.selectedResourceIds;

const params = new URLSearchParams();
params.append("datasets", selectedIds.toString());
Expand All @@ -181,7 +231,7 @@ export const compareMultipleDatasets = () => async (dispatch, getState) => {
type: TYPES.UNMATCHED_BENCHMARK_TYPES,
payload: "",
});
dispatch(parseChartData());
dispatch(parseChartData(response.data.benchmark));
}
} catch (error) {
if (
Expand Down
6 changes: 6 additions & 0 deletions dashboard/src/assets/constants/compareConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export const MAX_DATASETS_COMPARE = 5;
export const COLOR1 = "#8BC1F7";
export const COLOR2 = "#0066CC";
export const COLOR3 = "#519DE9";
export const COLOR4 = "#004B95";
export const COLOR5 = "#002F5D";
1 change: 1 addition & 0 deletions dashboard/src/assets/constants/toastConstants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export const DANGER = "danger";
export const ERROR_MSG = "Something went wrong!";
export const SUCCESS = "success";
export const WARNING = "warning";
2 changes: 2 additions & 0 deletions dashboard/src/modules/components/ComparisonComponent/ChartGallery.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
Chart as ChartJS,
Legend,
LinearScale,
SubTitle,
Title,
Tooltip,
} from "chart.js";
Expand All @@ -20,6 +21,7 @@ ChartJS.register(
Title,
Tooltip,
Legend,
SubTitle,
CategoryScale,
LinearScale
);
Expand Down
4 changes: 2 additions & 2 deletions dashboard/src/modules/components/ComparisonComponent/ChartModal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ const ChartModal = (props) => {
const currIndex = props.dataToPlot.findIndex(
(item) => item.data.id === activeChart.data.id
);
const prevId = props.dataToPlot[currIndex - 1]?.data?.id;
const nextId = props.dataToPlot[currIndex + 1]?.data?.id;
const prevId = props.dataToPlot[currIndex - 1]?.data.id;
const nextId = props.dataToPlot[currIndex + 1]?.data.id;
return (
<Modal
variant={ModalVariant.large}
Expand Down
35 changes: 12 additions & 23 deletions dashboard/src/modules/components/ComparisonComponent/PanelContent.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,7 @@ const PanelConent = () => {
if (searchValue === "") {
return true;
}
let input;
try {
input = new RegExp(searchValue, "i");
} catch (err) {
input = new RegExp(
searchValue.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"),
"i"
);
}
return item.name.search(input) >= 0;
return item.name.search(searchValue) >= 0;
},
[searchValue]
);
Expand All @@ -43,19 +34,17 @@ const PanelConent = () => {
<div className="datasets-container">
{isCompareSwitchChecked ? (
<div className="dataset-list-checkbox">
{filteredDatasets.map((item) => {
return (
<Checkbox
key={item.resource_id}
label={item.name}
id={item.resource_id}
isChecked={selectedResourceIds?.includes(item.resource_id)}
onChange={(checked) =>
dispatch(setSelectedId(checked, item.resource_id))
}
/>
);
})}
{filteredDatasets.map((item) => (
<Checkbox
key={item.resource_id}
label={item.name}
id={item.resource_id}
isChecked={selectedResourceIds?.includes(item.resource_id)}
onChange={(checked) =>
dispatch(setSelectedId(checked, item.resource_id))
}
/>
))}
</div>
) : (
<List isBordered>
Expand Down
2 changes: 1 addition & 1 deletion dashboard/src/reducers/comparisonReducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const ComparisonReducer = (state = initialState, action = {}) => {
case TYPES.TOGGLE_COMPARE_SWITCH:
return {
...state,
isCompareSwitchChecked: payload,
isCompareSwitchChecked: !state.isCompareSwitchChecked,
};
case TYPES.SET_SELECTED_RESOURCE_ID:
return {
Expand Down
2 changes: 1 addition & 1 deletion server/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ flask-restful>=0.3.9
flask-sqlalchemy
gunicorn
humanize
pquisby==0.0.17
pquisby
Copy link
Member

Choose a reason for hiding this comment

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

While this is OK, given the inconsistency we've seen in pquisby PyPi packages, I think I'd prefer to see pquisby==0.0.25 if that's what we're supporting for uperf and fio visualization. We can change the version later as necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Are we sure that we want to let this "float"?

Given recent experiences, I think we should peg this at a "known good" version.

Copy link
Member

Choose a reason for hiding this comment

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

I wondered at Webb's duplicate of my comment, but when I just started re-reviewing today I realized that I never actually published that earlier comment. Ooops!

So I'll just add here that pquisby is now at 0.0.27, vs 0.0.25 when I wrote that earlier comment, and I have no idea what's changed. Does it still work? I really do think we should be deliberately managing this dependency rather than letting it float.

psycopg2
pyesbulk>=2.0.1
PyJwt[crypto]
Expand Down