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: fields required in all groups and nodes requests #1515

Merged
merged 1 commit into from
Oct 23, 2024
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
1 change: 1 addition & 0 deletions src/containers/Storage/Storage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export const Storage = ({database, viewContext, nodeId, groupId, pDiskId}: Stora
groupId,
pDiskId,
shouldUseGroupsHandler: groupsHandlerAvailable,
fieldsRequired: 'all',
Copy link
Member Author

@artemmufazalov artemmufazalov Oct 22, 2024

Choose a reason for hiding this comment

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

It's possible to request data based on columns for not paginated storage. However, there is a problem: when you update columns, needed data fields are updated and data reloaded. For paginated table it isn't the issue, because it loads only 50 entities, but for not paginated tables requests take to long to load, so we need to avoid data reloads if possible.

I expect, that soon enough we will fix all issues with PaginatedTable and could get rid of DataTable in Storage

},
{
skip: !isGroups || !capabilitiesLoaded,
Expand Down
4 changes: 3 additions & 1 deletion src/containers/StorageGroupPage/StorageGroupPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ export function StorageGroupPage() {
const shouldUseGroupsHandler = useStorageGroupsHandlerAvailable();
const capabilitiesLoaded = useCapabilitiesLoaded();
const groupQuery = storageApi.useGetStorageGroupsInfoQuery(
valueIsDefined(groupId) ? {groupId, shouldUseGroupsHandler, with: 'all'} : skipToken,
valueIsDefined(groupId)
? {groupId, shouldUseGroupsHandler, with: 'all', fieldsRequired: 'all'}
Copy link
Member Author

Choose a reason for hiding this comment

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

Only 1 group is requested here and we need all its data

: skipToken,
{
pollingInterval: autoRefreshInterval,
skip: !capabilitiesLoaded,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,27 @@ import {
getHostColumn,
getNodeIdColumn,
} from '../../../../../components/nodesColumns/columns';
import {NODES_COLUMNS_WIDTH_LS_KEY} from '../../../../../components/nodesColumns/constants';
import {
NODES_COLUMNS_TO_DATA_FIELDS,
NODES_COLUMNS_WIDTH_LS_KEY,
} from '../../../../../components/nodesColumns/constants';
import type {GetNodesColumnsParams} from '../../../../../components/nodesColumns/types';
import {nodesApi} from '../../../../../store/reducers/nodes/nodes';
import type {NodesPreparedEntity} from '../../../../../store/reducers/nodes/types';
import {TENANT_DIAGNOSTICS_TABS_IDS} from '../../../../../store/reducers/tenant/constants';
import type {AdditionalNodesProps} from '../../../../../types/additionalProps';
import type {NodesRequiredField} from '../../../../../types/api/nodes';
import {TENANT_OVERVIEW_TABLES_LIMIT} from '../../../../../utils/constants';
import {useAutoRefreshInterval, useSearchQuery} from '../../../../../utils/hooks';
import {getRequiredDataFields} from '../../../../../utils/tableUtils/getRequiredDataFields';
import {TenantTabsGroups, getTenantPath} from '../../../TenantPages';
import {TenantOverviewTableLayout} from '../TenantOverviewTableLayout';
import {getSectionTitle} from '../getSectionTitle';
import i18n from '../i18n';

export function getTopNodesByCpuColumns(
params: GetNodesColumnsParams,
): Column<NodesPreparedEntity>[] {
): [Column<NodesPreparedEntity>[], NodesRequiredField[]] {
const hostColumn = {...getHostColumn<NodesPreparedEntity>(params), width: undefined};

const columns = [
Expand All @@ -29,10 +34,15 @@ export function getTopNodesByCpuColumns(
hostColumn,
];

return columns.map((column) => ({
const preparedColumns = columns.map((column) => ({
...column,
sortable: false,
}));

const columnsIds = preparedColumns.map((column) => column.name);
const dataFieldsRequired = getRequiredDataFields(columnsIds, NODES_COLUMNS_TO_DATA_FIELDS);

return [preparedColumns, dataFieldsRequired];
}

interface TopNodesByCpuProps {
Expand All @@ -44,7 +54,7 @@ export function TopNodesByCpu({tenantName, additionalNodesProps}: TopNodesByCpuP
const query = useSearchQuery();

const [autoRefreshInterval] = useAutoRefreshInterval();
const columns = getTopNodesByCpuColumns({
const [columns, fieldsRequired] = getTopNodesByCpuColumns({
getNodeRef: additionalNodesProps?.getNodeRef,
database: tenantName,
});
Expand All @@ -55,6 +65,8 @@ export function TopNodesByCpu({tenantName, additionalNodesProps}: TopNodesByCpuP
type: 'any',
sort: '-CPU',
limit: TENANT_OVERVIEW_TABLES_LIMIT,
tablets: false,
fieldsRequired,
},
{pollingInterval: autoRefreshInterval},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,27 @@ import {
getNodeIdColumn,
getVersionColumn,
} from '../../../../../components/nodesColumns/columns';
import {NODES_COLUMNS_WIDTH_LS_KEY} from '../../../../../components/nodesColumns/constants';
import {
NODES_COLUMNS_TO_DATA_FIELDS,
NODES_COLUMNS_WIDTH_LS_KEY,
} from '../../../../../components/nodesColumns/constants';
import type {GetNodesColumnsParams} from '../../../../../components/nodesColumns/types';
import {nodesApi} from '../../../../../store/reducers/nodes/nodes';
import type {NodesPreparedEntity} from '../../../../../store/reducers/nodes/types';
import {TENANT_DIAGNOSTICS_TABS_IDS} from '../../../../../store/reducers/tenant/constants';
import type {AdditionalNodesProps} from '../../../../../types/additionalProps';
import type {NodesRequiredField} from '../../../../../types/api/nodes';
import {TENANT_OVERVIEW_TABLES_LIMIT} from '../../../../../utils/constants';
import {useAutoRefreshInterval, useSearchQuery} from '../../../../../utils/hooks';
import {getRequiredDataFields} from '../../../../../utils/tableUtils/getRequiredDataFields';
import {TenantTabsGroups, getTenantPath} from '../../../TenantPages';
import {TenantOverviewTableLayout} from '../TenantOverviewTableLayout';
import {getSectionTitle} from '../getSectionTitle';
import i18n from '../i18n';

function getTopNodesByLoadColumns(params: GetNodesColumnsParams): Column<NodesPreparedEntity>[] {
function getTopNodesByLoadColumns(
params: GetNodesColumnsParams,
): [Column<NodesPreparedEntity>[], NodesRequiredField[]] {
const hostColumn = {
...getHostColumn<NodesPreparedEntity>(params),
width: undefined,
Expand All @@ -32,10 +39,15 @@ function getTopNodesByLoadColumns(params: GetNodesColumnsParams): Column<NodesPr
getVersionColumn<NodesPreparedEntity>(),
];

return columns.map((column) => ({
const preparedColumns = columns.map((column) => ({
...column,
sortable: false,
}));

const columnsIds = preparedColumns.map((column) => column.name);
const dataFieldsRequired = getRequiredDataFields(columnsIds, NODES_COLUMNS_TO_DATA_FIELDS);

return [preparedColumns, dataFieldsRequired];
}

interface TopNodesByLoadProps {
Expand All @@ -47,7 +59,7 @@ export function TopNodesByLoad({tenantName, additionalNodesProps}: TopNodesByLoa
const query = useSearchQuery();

const [autoRefreshInterval] = useAutoRefreshInterval();
const columns = getTopNodesByLoadColumns({
const [columns, fieldsRequired] = getTopNodesByLoadColumns({
getNodeRef: additionalNodesProps?.getNodeRef,
database: tenantName,
});
Expand All @@ -58,6 +70,8 @@ export function TopNodesByLoad({tenantName, additionalNodesProps}: TopNodesByLoa
type: 'any',
sort: '-LoadAverage',
limit: TENANT_OVERVIEW_TABLES_LIMIT,
tablets: false,
fieldsRequired,
},
{pollingInterval: autoRefreshInterval},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,27 @@ import {
getTabletsColumn,
getUptimeColumn,
} from '../../../../../components/nodesColumns/columns';
import {NODES_COLUMNS_WIDTH_LS_KEY} from '../../../../../components/nodesColumns/constants';
import {
NODES_COLUMNS_TO_DATA_FIELDS,
NODES_COLUMNS_WIDTH_LS_KEY,
} from '../../../../../components/nodesColumns/constants';
import type {GetNodesColumnsParams} from '../../../../../components/nodesColumns/types';
import {nodesApi} from '../../../../../store/reducers/nodes/nodes';
import type {NodesPreparedEntity} from '../../../../../store/reducers/nodes/types';
import {TENANT_DIAGNOSTICS_TABS_IDS} from '../../../../../store/reducers/tenant/constants';
import type {AdditionalNodesProps} from '../../../../../types/additionalProps';
import type {NodesRequiredField} from '../../../../../types/api/nodes';
import {TENANT_OVERVIEW_TABLES_LIMIT} from '../../../../../utils/constants';
import {useAutoRefreshInterval, useSearchQuery} from '../../../../../utils/hooks';
import {getRequiredDataFields} from '../../../../../utils/tableUtils/getRequiredDataFields';
import {TenantTabsGroups, getTenantPath} from '../../../TenantPages';
import {TenantOverviewTableLayout} from '../TenantOverviewTableLayout';
import {getSectionTitle} from '../getSectionTitle';
import i18n from '../i18n';

function getTopNodesByMemoryColumns(params: GetNodesColumnsParams): Column<NodesPreparedEntity>[] {
function getTopNodesByMemoryColumns(
params: GetNodesColumnsParams,
): [Column<NodesPreparedEntity>[], NodesRequiredField[]] {
const memoryColumn = {
...getMemoryColumn<NodesPreparedEntity>(),
header: i18n('column-header.process'),
Expand All @@ -40,10 +47,15 @@ function getTopNodesByMemoryColumns(params: GetNodesColumnsParams): Column<Nodes
getTabletsColumn<NodesPreparedEntity>(params),
];

return columns.map((column) => ({
const preparedColumns = columns.map((column) => ({
...column,
sortable: false,
}));

const columnsIds = preparedColumns.map((column) => column.name);
const dataFieldsRequired = getRequiredDataFields(columnsIds, NODES_COLUMNS_TO_DATA_FIELDS);

return [preparedColumns, dataFieldsRequired];
}

interface TopNodesByMemoryProps {
Expand All @@ -55,7 +67,7 @@ export function TopNodesByMemory({tenantName, additionalNodesProps}: TopNodesByM
const query = useSearchQuery();

const [autoRefreshInterval] = useAutoRefreshInterval();
const columns = getTopNodesByMemoryColumns({
const [columns, fieldsRequired] = getTopNodesByMemoryColumns({
getNodeRef: additionalNodesProps?.getNodeRef,
database: tenantName,
});
Expand All @@ -67,6 +79,7 @@ export function TopNodesByMemory({tenantName, additionalNodesProps}: TopNodesByM
tablets: true,
sort: '-Memory',
limit: TENANT_OVERVIEW_TABLES_LIMIT,
fieldsRequired,
},
{pollingInterval: autoRefreshInterval},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,30 @@ import {
} from '../../../../../store/reducers/capabilities/hooks';
import {storageApi} from '../../../../../store/reducers/storage/storage';
import {TENANT_DIAGNOSTICS_TABS_IDS} from '../../../../../store/reducers/tenant/constants';
import type {GroupsRequiredField} from '../../../../../types/api/storage';
import {TENANT_OVERVIEW_TABLES_LIMIT} from '../../../../../utils/constants';
import {useAutoRefreshInterval, useSearchQuery} from '../../../../../utils/hooks';
import {getRequiredDataFields} from '../../../../../utils/tableUtils/getRequiredDataFields';
import {getStorageTopGroupsColumns} from '../../../../Storage/StorageGroups/columns/columns';
import {STORAGE_GROUPS_COLUMNS_WIDTH_LS_KEY} from '../../../../Storage/StorageGroups/columns/constants';
import {
GROUPS_COLUMNS_TO_DATA_FIELDS,
STORAGE_GROUPS_COLUMNS_WIDTH_LS_KEY,
} from '../../../../Storage/StorageGroups/columns/constants';
import type {StorageGroupsColumn} from '../../../../Storage/StorageGroups/columns/types';
import {TenantTabsGroups, getTenantPath} from '../../../TenantPages';
import {TenantOverviewTableLayout} from '../TenantOverviewTableLayout';
import {getSectionTitle} from '../getSectionTitle';
import i18n from '../i18n';

function getColumns(): [StorageGroupsColumn[], GroupsRequiredField[]] {
const preparedColumns = getStorageTopGroupsColumns();

const columnsIds = preparedColumns.map((column) => column.name);
const dataFieldsRequired = getRequiredDataFields(columnsIds, GROUPS_COLUMNS_TO_DATA_FIELDS);

return [preparedColumns, dataFieldsRequired];
}

interface TopGroupsProps {
tenant?: string;
}
Expand All @@ -24,7 +39,7 @@ export function TopGroups({tenant}: TopGroupsProps) {
const groupsHandlerAvailable = useStorageGroupsHandlerAvailable();
const [autoRefreshInterval] = useAutoRefreshInterval();

const columns = getStorageTopGroupsColumns();
const [columns, fieldsRequired] = getColumns();

const {currentData, isFetching, error} = storageApi.useGetStorageGroupsInfoQuery(
{
Expand All @@ -33,6 +48,7 @@ export function TopGroups({tenant}: TopGroupsProps) {
with: 'all',
limit: TENANT_OVERVIEW_TABLES_LIMIT,
shouldUseGroupsHandler: groupsHandlerAvailable,
fieldsRequired,
},
{
pollingInterval: autoRefreshInterval,
Expand Down
2 changes: 1 addition & 1 deletion src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export class YdbEmbeddedAPI extends AxiosWrapper {
);
}
getStorageGroups(
{nodeId, pDiskId, groupId, fieldsRequired = 'all', filter, ...params}: GroupsRequestParams,
{nodeId, pDiskId, groupId, fieldsRequired, filter, ...params}: GroupsRequestParams,
{concurrentId, signal}: AxiosOptions = {},
) {
const preparedNodeId = Array.isArray(nodeId)
Expand Down
2 changes: 1 addition & 1 deletion src/utils/tableUtils/getRequiredDataFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@ export function getRequiredDataFields<ColumnId extends string, RequiredField ext
return fields;
}, new Set<RequiredField>());

return Array.from(requiredFieldsSet);
return Array.from(requiredFieldsSet).sort();
Copy link
Member Author

Choose a reason for hiding this comment

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

Sort to ensure no additional requests when order changes.

Example:
Columns [DiskSpace, Read], data fields [State, Read]
Columns [Read, State], data fields [Read, State]

Although it's the same data, it will be requested twice, because of different state keys ("fieldsRequired:State,Read" vs "fieldsRequired:Read,State"

}
Loading