From 9ba041af8c457d48dd48261cb44a430807c15d4a Mon Sep 17 00:00:00 2001 From: Gordon Smith Date: Thu, 5 Feb 2026 16:39:05 +0000 Subject: [PATCH] HPCC-35694 Refactor file size + compression ratio - Expose single field 'FileSize' for physical File Size. Regardless of compression status this field has the actual physical size on disk. - Add FileSize field to DFUQuery response to hold DFUSFSize. - Update ECL Watch to use FileSize when present. - Update DFULogicalFile structure and all services using it to version 1.69. - Update unittests to return all DFU fields needed to pass new tests. Signed-off-by: Gordon Smith Signed-off-by: Terrence Asselin --- dali/base/dadfs.cpp | 13 ++++++--- esp/scm/ws_dfu.ecm | 4 +-- esp/scm/ws_dfuXref.ecm | 2 +- esp/scm/ws_dfu_common.ecm | 1 + esp/services/ws_dfu/ws_dfuHelpers.cpp | 20 +++++++++++++ esp/services/ws_dfu/ws_dfuService.cpp | 1 + esp/src/eclwatch/DFUQueryWidget.js | 4 +-- esp/src/src-react/components/Files.tsx | 29 ++++++++++++------- .../src-react/components/IndexFileSummary.tsx | 6 ++-- .../components/LogicalFileSummary.tsx | 6 ++-- .../components/MetricsPropertiesTables.tsx | 2 +- esp/src/src-react/hooks/file.ts | 28 ++++++++++++++++++ esp/src/src/Utility.ts | 4 +-- esp/src/tests/v9-files.spec.ts | 4 +-- testing/unittests/dalitests.cpp | 13 ++++++++- 15 files changed, 105 insertions(+), 32 deletions(-) diff --git a/dali/base/dadfs.cpp b/dali/base/dadfs.cpp index 1557bdb98c0..a19fbf48261 100644 --- a/dali/base/dadfs.cpp +++ b/dali/base/dadfs.cpp @@ -14255,11 +14255,16 @@ static IPropertyTreeIterator *deserializeFileAttrIterator(MemoryBuffer& mb, unsi if (options.includeField(DFUQResultField::size)) { - // JCSMORE - I am not sure what the point of this is, with or without it, a blank @size does not affect sort order - // and EclWatch seems to use the @size (DFUQResultField::origsize) for size column anyway - // See special handling in SerializeFileAttrOptions::readFields() to include origsize if size is requested + // Size field is notionally the size on disk const char *propName = getDFUQResultFieldName(DFUQResultField::size); - attr->setPropInt64(propName, attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1));//Sort the files with empty size to front + attr->setPropInt64(propName, isCompressed(*attr) ? attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::compressedsize), -1) : attr->getPropInt64(getDFUQResultFieldName(DFUQResultField::origsize), -1)); + } + + if (options.includeField(DFUQResultField::origsize)) + { + const char *propName = getDFUQResultFieldName(DFUQResultField::origsize); + if (!attr->hasProp(propName)) + attr->setPropInt64(propName, -1); } if (options.includeField(DFUQResultField::recordcount)) diff --git a/esp/scm/ws_dfu.ecm b/esp/scm/ws_dfu.ecm index 32f7e1e71f4..31648f5caa7 100644 --- a/esp/scm/ws_dfu.ecm +++ b/esp/scm/ws_dfu.ecm @@ -1037,8 +1037,8 @@ ESPresponse [version("1.68"), exceptions_inline, nil_remove] DFUFileRenameRespon // =========================================================================== ESPservice [ auth_feature("DEFERRED"), - version("1.68"), - default_client_version("1.68"), + version("1.69"), + default_client_version("1.69"), noforms, exceptions_inline("./smc_xslt/exceptions.xslt")] WsDfu { diff --git a/esp/scm/ws_dfuXref.ecm b/esp/scm/ws_dfuXref.ecm index 4b1f8ddf8dc..3b0844e956d 100644 --- a/esp/scm/ws_dfuXref.ecm +++ b/esp/scm/ws_dfuXref.ecm @@ -228,7 +228,7 @@ ESPresponse [exceptions_inline] DFUXRefUnusedFilesResponse }; // =========================================================================== -ESPservice [version("1.04"), default_client_version("1.04"), auth_feature("DEFERRED"), exceptions_inline("./smc_xslt/exceptions.xslt")] WsDFUXRef +ESPservice [version("1.69"), default_client_version("1.69"), auth_feature("DEFERRED"), exceptions_inline("./smc_xslt/exceptions.xslt")] WsDFUXRef { ///ESPmethod [resp_xsl_default("./smc_xslt/xref_main.xslt")] DFUXRefList(DFUXRefListRequest, DFUXRefListResponse); ESPmethod [resp_xsl_default("/esp/xslt/xref_main.xslt")] DFUXRefList(DFUXRefListRequest, DFUXRefListResponse); diff --git a/esp/scm/ws_dfu_common.ecm b/esp/scm/ws_dfu_common.ecm index 6d2fb22251a..225f1c7efed 100644 --- a/esp/scm/ws_dfu_common.ecm +++ b/esp/scm/ws_dfu_common.ecm @@ -59,4 +59,5 @@ ESPStruct DFULogicalFile [min_ver("1.63"), nil_remove] int64 MaxSkew; [min_ver("1.63"), nil_remove] int64 MinSkewPart; [min_ver("1.63"), nil_remove] int64 MaxSkewPart; + [min_ver("1.69")] int64 FileSize; }; diff --git a/esp/services/ws_dfu/ws_dfuHelpers.cpp b/esp/services/ws_dfu/ws_dfuHelpers.cpp index 0ebaa514c8d..bcb88e8109d 100644 --- a/esp/services/ws_dfu/ws_dfuHelpers.cpp +++ b/esp/services/ws_dfu/ws_dfuHelpers.cpp @@ -197,6 +197,26 @@ bool WsDFUHelpers::addToLogicalFileList(IPropertyTree& file, const char* nodeGro lFile->setMinSkewPart(file.getPropInt64(getDFUQResultFieldName(DFUQResultField::minSkewPart))); } + if (version >= 1.68) + { + if (file.hasProp(getDFUQResultFieldName(DFUQResultField::size))) + lFile->setFileSize(file.getPropInt64(getDFUQResultFieldName(DFUQResultField::size))); + else + { + if (isFileCompressed) + { + if (file.hasProp(getDFUQResultFieldName(DFUQResultField::compressedsize))) + lFile->setFileSize(file.getPropInt64(getDFUQResultFieldName(DFUQResultField::compressedsize))); + else if (isKeyFile) + lFile->setFileSize(size); + } + else + { + lFile->setFileSize(size); + } + } + } + logicalFiles.append(*lFile.getClear()); } catch(IException* e) diff --git a/esp/services/ws_dfu/ws_dfuService.cpp b/esp/services/ws_dfu/ws_dfuService.cpp index b553c986091..abfb621f6ec 100644 --- a/esp/services/ws_dfu/ws_dfuService.cpp +++ b/esp/services/ws_dfu/ws_dfuService.cpp @@ -3769,6 +3769,7 @@ void CWsDfuEx::setDFUQuerySortOrder(IEspDFUQueryRequest& req, StringBuffer& sort static const std::unordered_map legacyMappings = { {"FileSize", "@DFUSFsize"}, + {"CompressedFileSize", "@compressedSize"}, {"ContentType", "@kind"}, {"IsCompressed", "@compressed"}, {"Records", "@recordcount"}, diff --git a/esp/src/eclwatch/DFUQueryWidget.js b/esp/src/eclwatch/DFUQueryWidget.js index d92cc2945da..7a7e5d2f717 100644 --- a/esp/src/eclwatch/DFUQueryWidget.js +++ b/esp/src/eclwatch/DFUQueryWidget.js @@ -737,7 +737,7 @@ define([ label: nlsHPCC.MinSkew, width: 60, formatter: function (value, row) { if (value) { - return "-" + Utility.formatDecimal(value / 100) + "%"; + return Utility.formatDecimal(value / 100, "-", "%"); } return ""; } @@ -746,7 +746,7 @@ define([ label: nlsHPCC.MaxSkew, width: 60, formatter: function (value, row) { if (value) { - return Utility.formatDecimal(value / 100) + "%"; + return Utility.formatDecimal(value / 100, "", "%"); } return ""; } diff --git a/esp/src/src-react/components/Files.tsx b/esp/src/src-react/components/Files.tsx index 396559b626a..7c57759f8b0 100644 --- a/esp/src/src-react/components/Files.tsx +++ b/esp/src/src-react/components/Files.tsx @@ -9,6 +9,7 @@ import { QuerySortItem } from "src/store/Store"; import nlsHPCC from "src/nlsHPCC"; import { useConfirm } from "../hooks/confirm"; import { useMyAccount } from "../hooks/user"; +import { formatCompression } from "../hooks/file"; import { HolyGrail } from "../layouts/HolyGrail"; import { pushParams } from "../util/history"; import { FluentPagedGrid, FluentPagedFooter, useCopyButtons, useFluentStoreState, FluentColumns } from "./controls/Grid"; @@ -102,7 +103,6 @@ export const Files: React.FunctionComponent = ({ page = 1, store }) => { - const hasFilter = React.useMemo(() => Object.keys(filter).length > 0, [filter]); const [showFilter, setShowFilter] = React.useState(false); @@ -210,28 +210,35 @@ export const Files: React.FunctionComponent = ({ csvFormatter: (value, row) => row.IntRecordCount, }, FileSize: { - label: nlsHPCC.Size, + label: nlsHPCC.FileSize, formatter: (value, row) => { - return Utility.convertedSize(row.IntSize); + if (row.FileSize !== undefined) { + return Utility.convertedSize(row.FileSize); + } + return Utility.convertedSize(row.IsCompressed ? row.CompressedFileSize : row.IntSize); + }, + csvFormatter: (value, row) => { + if (row.FileSize !== undefined) { + return row.FileSize; + } + return row.IsCompressed ? row.CompressedFileSize : row.IntSize; }, - csvFormatter: (value, row) => row.IntSize, }, - CompressedFileSizeString: { - label: nlsHPCC.CompressedSize, + Compression: { + label: nlsHPCC.Compression, sortable: false, formatter: (value, row) => { - return Utility.convertedSize(row.CompressedFileSize); - }, - csvFormatter: (value, row) => row.CompressedFileSize, + return formatCompression(row); + } }, Parts: { label: nlsHPCC.Parts, width: 40, }, MinSkew: { - label: nlsHPCC.MinSkew, width: 60, formatter: (value, row) => value ? `-${Utility.formatDecimal(value / 100)}%` : "" + label: nlsHPCC.MinSkew, width: 60, formatter: (value, row) => value ? `${Utility.formatDecimal(value / 100, "-", "%")}` : "" }, MaxSkew: { - label: nlsHPCC.MaxSkew, width: 60, formatter: (value, row) => value ? `${Utility.formatDecimal(value / 100)}%` : "" + label: nlsHPCC.MaxSkew, width: 60, formatter: (value, row) => value ? `${Utility.formatDecimal(value / 100, "", "%")}` : "" }, Accessed: { label: uiState.isUTC ? nlsHPCC.LastAccessed : nlsHPCC.LastAccessedLocalTime, diff --git a/esp/src/src-react/components/IndexFileSummary.tsx b/esp/src/src-react/components/IndexFileSummary.tsx index 5f801c824ae..32184d5da2a 100644 --- a/esp/src/src-react/components/IndexFileSummary.tsx +++ b/esp/src/src-react/components/IndexFileSummary.tsx @@ -269,21 +269,21 @@ export const IndexFileSummary: React.FunctionComponent = label: nlsHPCC.File, originalSize: Utility.convertedSize(file?.FileSizeInt64), diskSize: Utility.convertedSize(file?.CompressedFileSize || file?.FileSizeInt64), - percentCompressed: ((file?.CompressedFileSize && file?.FileSizeInt64) ? Utility.formatDecimal(100 * file?.CompressedFileSize / file?.FileSizeInt64) : 0) + "%", + percentCompressed: ((file?.CompressedFileSize && file?.FileSizeInt64) ? Utility.formatDecimal(100 * file?.CompressedFileSize / file?.FileSizeInt64, "", "%") : ""), memorySize: (file?.ExtendedIndexInfo?.SizeMemoryBranches && file?.ExtendedIndexInfo?.SizeMemoryLeaves) ? Utility.convertedSize(file?.ExtendedIndexInfo?.SizeMemoryBranches + file?.ExtendedIndexInfo?.SizeMemoryLeaves) : "" }, { label: nlsHPCC.Branches, originalSize: Utility.convertedSize(file?.ExtendedIndexInfo?.SizeOriginalBranches) ?? "", diskSize: Utility.convertedSize(file?.ExtendedIndexInfo?.SizeDiskBranches) ?? "", - percentCompressed: file?.ExtendedIndexInfo?.BranchCompressionPercent ? Utility.formatDecimal(file.ExtendedIndexInfo.BranchCompressionPercent) + "%" : "", + percentCompressed: file?.ExtendedIndexInfo?.BranchCompressionPercent ? Utility.formatDecimal(file.ExtendedIndexInfo.BranchCompressionPercent, "", "%") : "", memorySize: Utility.convertedSize(file?.ExtendedIndexInfo?.SizeMemoryBranches) ?? "" }, { label: nlsHPCC.Data, originalSize: Utility.convertedSize(file?.ExtendedIndexInfo?.SizeOriginalData) ?? "", diskSize: (file?.ExtendedIndexInfo?.SizeDiskLeaves !== undefined && file?.ExtendedIndexInfo?.SizeDiskBlobs !== undefined) ? Utility.convertedSize(file?.ExtendedIndexInfo?.SizeDiskLeaves + file?.ExtendedIndexInfo?.SizeDiskBlobs) : "", - percentCompressed: file?.ExtendedIndexInfo?.DataCompressionPercent ? Utility.formatDecimal(file.ExtendedIndexInfo.DataCompressionPercent) + "%" : "", + percentCompressed: file?.ExtendedIndexInfo?.DataCompressionPercent ? Utility.formatDecimal(file.ExtendedIndexInfo.DataCompressionPercent, "", "%") : "", memorySize: Utility.convertedSize(file?.ExtendedIndexInfo?.SizeMemoryLeaves) ?? "" } ]} diff --git a/esp/src/src-react/components/LogicalFileSummary.tsx b/esp/src/src-react/components/LogicalFileSummary.tsx index 311ec9d6518..03aa8153f4c 100644 --- a/esp/src/src-react/components/LogicalFileSummary.tsx +++ b/esp/src/src-react/components/LogicalFileSummary.tsx @@ -7,7 +7,7 @@ import { formatCost } from "src/Session"; import * as Utility from "src/Utility"; import { getStateImageName, IFile } from "src/ESPLogicalFile"; import { useConfirm } from "../hooks/confirm"; -import { useFile } from "../hooks/file"; +import { formatCompression, useFile } from "../hooks/file"; import { useMyAccount } from "../hooks/user"; import { ShortVerticalDivider } from "./Common"; import { TableGroup } from "./forms/Groups"; @@ -218,11 +218,11 @@ export const LogicalFileSummary: React.FunctionComponent { const lIdx = sortByColumns.indexOf(l[0]); diff --git a/esp/src/src-react/hooks/file.ts b/esp/src/src-react/hooks/file.ts index 72b33f61462..bfeaccbbd4e 100644 --- a/esp/src/src-react/hooks/file.ts +++ b/esp/src/src-react/hooks/file.ts @@ -1,11 +1,39 @@ import * as React from "react"; import { LogicalFile, WsDfu } from "@hpcc-js/comms"; import { scopedLogger } from "@hpcc-js/util"; +import * as Utility from "src/Utility"; import { singletonDebounce } from "../util/throttle"; import { useCounter } from "./util"; const logger = scopedLogger("../hooks/file.ts"); +export interface LogicalFileEx extends LogicalFile { + IntSize: number; +} + +function isLogicalFileEx(file: LogicalFile): file is LogicalFileEx { + return (file as LogicalFileEx).IntSize !== undefined; +} + +function formatRatio(isCompressed: boolean, compressedSize: number, totalSize: number): string { + if (!isCompressed || totalSize <= 0) return ""; + const ratio = compressedSize / totalSize; + if (!isFinite(ratio)) return ""; + return Utility.formatDecimal(100 - ratio * 100, "", "%"); +} + +function formatCompressionEx(file?: LogicalFileEx): string { + if (!file) return ""; + return formatRatio(file.IsCompressed, file.CompressedFileSize, file.IntSize); +} + +export function formatCompression(file?: LogicalFile): string { + if (!file) return ""; + if (file.isSuperfile) return ""; + if (isLogicalFileEx(file)) return formatCompressionEx(file); + return formatRatio(file.IsCompressed, file.CompressedFileSize, file.FileSizeInt64); +} + interface useFileResponse { file: LogicalFile; protectedBy: WsDfu.DFUFileProtect[]; diff --git a/esp/src/src/Utility.ts b/esp/src/src/Utility.ts index 56ef42c7414..4f641062e9f 100644 --- a/esp/src/src/Utility.ts +++ b/esp/src/src/Utility.ts @@ -1191,10 +1191,10 @@ export function deleteCookie(name: string) { const d3FormatDecimal = d3Format(",.2f"); const d3FormatInt = d3Format(",.0f"); -export function formatDecimal(num: number): string { +export function formatDecimal(num: number, prefix: string = "", postfix: string = ""): string { if (!num) return ""; if (isNaN(num)) return num.toString(); - return d3FormatDecimal(num); + return prefix + d3FormatDecimal(num) + postfix; } export function formatNum(num: number): string { diff --git a/esp/src/tests/v9-files.spec.ts b/esp/src/tests/v9-files.spec.ts index d75aa7b633f..005da44c3d9 100644 --- a/esp/src/tests/v9-files.spec.ts +++ b/esp/src/tests/v9-files.spec.ts @@ -25,8 +25,8 @@ test.describe("V9 Files - Logical Files", () => { await expect(page.getByText("Description")).toBeVisible(); await expect(page.getByText("Cluster", { exact: true })).toBeVisible(); await expect(page.getByText("Records")).toBeVisible(); - await expect(page.getByText("Size", { exact: true })).toBeVisible(); - await expect(page.getByText("Compressed Size")).toBeVisible(); + await expect(page.getByText("File Size")).toBeVisible(); + await expect(page.getByText("Compression")).toBeVisible(); await expect(page.getByText("Parts")).toBeVisible(); await expect(page.getByText("Min Skew")).toBeVisible(); await expect(page.getByText("Max Skew")).toBeVisible(); diff --git a/testing/unittests/dalitests.cpp b/testing/unittests/dalitests.cpp index 4b18ddad529..f4403ee0619 100644 --- a/testing/unittests/dalitests.cpp +++ b/testing/unittests/dalitests.cpp @@ -2683,6 +2683,9 @@ class DaliDFSIteratorTests : public CppUnit::TestFixture attr.setProp("@job", attrValue); attr.setProp("@owner", attrValue); attr.setProp("@workunit", attrValue); + attr.setPropBool("@rowCompressed", true); + attr.setPropInt64("@compressedSize", 9); + attr.setPropInt64("@size", 17); } } @@ -2835,7 +2838,12 @@ class DaliDFSIteratorTests : public CppUnit::TestFixture filterBuf.append(DFUQFTspecial).append(DFUQFilterSeparator).append(DFUQSFFileType).append(DFUQFilterSeparator).append(DFUQFFTnonsuperfileonly).append(DFUQFilterSeparator); filterBuf.append(DFUQFTspecial).append(DFUQFilterSeparator).append(DFUQSFFileNameWithPrefix).append(DFUQFilterSeparator).append(wildName).append(DFUQFilterSeparator); - std::vector fields = {DFUQResultField::size, DFUQResultField::cost, DFUQResultField::term}; + // Must include all fields needed for sorting and inspection in loop below. + // + // rowCompressed and blockCompressed required for getLogicalFilesSorted to successfully run the isCompressed() + // test used when determining which size value to populate DFUQResultField::size with (compressed vs uncompressed). + // Alternately, could use DFUQResultField::includeAll to return all fields if the test expands in scope. + std::vector fields = {DFUQResultField::size, DFUQResultField::cost, DFUQResultField::rowCompressed, DFUQResultField::blockCompressed, DFUQResultField::term}; DFUQResultField sortOrder[2] = {DFUQResultField::job|DFUQResultField::reverse, DFUQResultField::term}; __int64 hint; @@ -2857,6 +2865,9 @@ class DaliDFSIteratorTests : public CppUnit::TestFixture CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: Missing cost attributes", costAttrsPresent); bool dirAttrsPresent = attrs.hasProp("@directory"); CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: directory attribute should NOT be present", !dirAttrsPresent); + CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: size field missing", attrs.hasProp("@DFUSFsize")); + CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: compressed size field missing", attrs.hasProp("@compressedSize")); + CPPUNIT_ASSERT_MESSAGE("testGetLogicalFilesSorted: size should use compressed size", attrs.getPropInt64("@DFUSFsize", -1) == attrs.getPropInt64("@compressedSize", -1)); } } };