Skip to content

Commit

Permalink
Fix bug in which Index Management data streams were being sorted inco…
Browse files Browse the repository at this point in the history
…rrectly by storage size. (#86204)

They were previously being sorted alphabetically using the humanized size value. Now they're sorted numerically by the raw bytes value.
  • Loading branch information
cjcenizal authored Dec 17, 2020
1 parent 5d2014d commit 7bac741
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 13 deletions.
Empty file removed data/.empty
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export interface DataStreamsTabTestBed extends TestBed<TestSubjects> {
clickEmptyPromptIndexTemplateLink: () => void;
clickIncludeStatsSwitch: () => void;
toggleViewFilterAt: (index: number) => void;
sortTableOnStorageSize: () => void;
clickReloadButton: () => void;
clickNameAt: (index: number) => void;
clickIndicesAt: (index: number) => void;
Expand Down Expand Up @@ -94,6 +95,14 @@ export const setup = async (overridingDependencies: any = {}): Promise<DataStrea
component.update();
};

const sortTableOnStorageSize = () => {
const { find, component } = testBed;
act(() => {
find('tableHeaderCell_storageSizeBytes_3.tableHeaderSortButton').simulate('click');
});
component.update();
};

const clickReloadButton = () => {
const { find } = testBed;
find('reloadButton').simulate('click');
Expand Down Expand Up @@ -205,6 +214,7 @@ export const setup = async (overridingDependencies: any = {}): Promise<DataStrea
clickEmptyPromptIndexTemplateLink,
clickIncludeStatsSwitch,
toggleViewFilterAt,
sortTableOnStorageSize,
clickReloadButton,
clickNameAt,
clickIndicesAt,
Expand Down Expand Up @@ -238,6 +248,7 @@ export const createDataStreamPayload = (dataStream: Partial<DataStream>): DataSt
health: 'green',
indexTemplateName: 'indexTemplate',
storageSize: '1b',
storageSizeBytes: 1,
maxTimeStamp: 420,
privileges: {
delete_index: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,21 @@ describe('Data Streams tab', () => {
createNonDataStreamIndex('non-data-stream-index'),
]);

const dataStreamForDetailPanel = createDataStreamPayload({ name: 'dataStream1' });
const dataStreamForDetailPanel = createDataStreamPayload({
name: 'dataStream1',
storageSize: '5b',
storageSizeBytes: 5,
});

setLoadDataStreamsResponse([
dataStreamForDetailPanel,
createDataStreamPayload({ name: 'dataStream2' }),
createDataStreamPayload({
name: 'dataStream2',
storageSize: '1kb',
storageSizeBytes: 1000,
}),
]);

setLoadDataStreamResponse(dataStreamForDetailPanel);

const indexTemplate = fixtures.getTemplate({ name: 'indexTemplate' });
Expand Down Expand Up @@ -181,8 +191,28 @@ describe('Data Streams tab', () => {
// The table renders with the stats columns though.
const { tableCellsValues } = table.getMetaData('dataStreamTable');
expect(tableCellsValues).toEqual([
['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '1b', '1', 'Delete'],
['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1b', '1', 'Delete'],
['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '5b', '1', 'Delete'],
['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1kb', '1', 'Delete'],
]);
});

test('sorting on stats sorts by bytes value instead of human readable value', async () => {
// Guards against regression of #86122.
const { actions, table, component } = testBed;

await act(async () => {
actions.clickIncludeStatsSwitch();
});
component.update();

actions.sortTableOnStorageSize();

// The table sorts by the underlying byte values in ascending order, instead of sorting by
// the human-readable string values.
const { tableCellsValues } = table.getMetaData('dataStreamTable');
expect(tableCellsValues).toEqual([
['', 'dataStream1', 'green', 'December 31st, 1969 7:00:00 PM', '5b', '1', 'Delete'],
['', 'dataStream2', 'green', 'December 31st, 1969 7:00:00 PM', '1kb', '1', 'Delete'],
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export function deserializeDataStream(dataStreamFromEs: DataStreamFromEs): DataS
template,
ilm_policy: ilmPolicyName,
store_size: storageSize,
store_size_bytes: storageSizeBytes,
maximum_timestamp: maxTimeStamp,
_meta,
privileges,
Expand All @@ -37,6 +38,7 @@ export function deserializeDataStream(dataStreamFromEs: DataStreamFromEs): DataS
indexTemplateName: template,
ilmPolicyName,
storageSize,
storageSizeBytes,
maxTimeStamp,
_meta,
privileges,
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/index_management/common/types/data_streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface DataStreamFromEs {
template: string;
ilm_policy?: string;
store_size?: string;
store_size_bytes?: number;
maximum_timestamp?: number;
privileges: PrivilegesFromEs;
hidden: boolean;
Expand All @@ -57,6 +58,7 @@ export interface DataStream {
indexTemplateName: string;
ilmPolicyName?: string;
storageSize?: string;
storageSizeBytes?: number;
maxTimeStamp?: number;
_meta?: Meta;
privileges: Privileges;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,14 @@ export const DataStreamTable: React.FunctionComponent<Props> = ({
});

columns.push({
field: 'storageSize',
field: 'storageSizeBytes',
name: i18n.translate('xpack.idxMgmt.dataStreamList.table.storageSizeColumnTitle', {
defaultMessage: 'Storage size',
}),
truncateText: true,
sortable: true,
render: (storageSizeBytes: DataStream['storageSizeBytes'], dataStream: DataStream) =>
dataStream.storageSize,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ interface PrivilegesFromEs {
interface StatsFromEs {
data_stream: string;
store_size: string;
store_size_bytes: number;
maximum_timestamp: number;
}

Expand All @@ -40,14 +41,15 @@ const enhanceDataStreams = ({

if (dataStreamsStats) {
// eslint-disable-next-line @typescript-eslint/naming-convention
const { store_size, maximum_timestamp } =
const { store_size, store_size_bytes, maximum_timestamp } =
dataStreamsStats.find(
({ data_stream: statsName }: { data_stream: string }) => statsName === dataStream.name
) || {};

enhancedDataStream = {
...enhancedDataStream,
store_size,
store_size_bytes,
maximum_timestamp,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ export default function ({ getService }: FtrProviderContext) {
await deleteComposableIndexTemplate(name);
};

const assertDataStreamStorageSizeExists = (storageSize: string) => {
// Storage size of a document doesn't like it would be deterministic (could vary depending
const assertDataStreamStorageSizeExists = (storageSize: string, storageSizeBytes: number) => {
// Storage size of a document doesn't look like it would be deterministic (could vary depending
// on how ES, Lucene, and the file system interact), so we'll just assert its presence and
// type.
expect(storageSize).to.be.ok();
expect(typeof storageSize).to.be('string');
expect(storageSizeBytes).to.be.ok();
expect(typeof storageSizeBytes).to.be('number');
};

describe('Data streams', function () {
Expand Down Expand Up @@ -120,10 +122,9 @@ export default function ({ getService }: FtrProviderContext) {
expect(testDataStream).to.be.ok();

// ES determines these values so we'll just echo them back.

const { name: indexName, uuid } = testDataStream!.indices[0];
const { storageSize, ...dataStreamWithoutStorageSize } = testDataStream!;
assertDataStreamStorageSizeExists(storageSize);
const { storageSize, storageSizeBytes, ...dataStreamWithoutStorageSize } = testDataStream!;
assertDataStreamStorageSizeExists(storageSize, storageSizeBytes);

expect(dataStreamWithoutStorageSize).to.eql({
name: testDataStreamName,
Expand Down Expand Up @@ -153,8 +154,8 @@ export default function ({ getService }: FtrProviderContext) {

// ES determines these values so we'll just echo them back.
const { name: indexName, uuid } = dataStream.indices[0];
const { storageSize, ...dataStreamWithoutStorageSize } = dataStream;
assertDataStreamStorageSizeExists(storageSize);
const { storageSize, storageSizeBytes, ...dataStreamWithoutStorageSize } = dataStream;
assertDataStreamStorageSizeExists(storageSize, storageSizeBytes);

expect(dataStreamWithoutStorageSize).to.eql({
name: testDataStreamName,
Expand Down

0 comments on commit 7bac741

Please sign in to comment.