Skip to content
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
4 changes: 2 additions & 2 deletions internal/cli/copy_catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ func TestCopyCatalog(t *testing.T) {

// ── 02 data list ────────────────────────────────────────────────────────────
sample := []push.DatasetInfo{
{Name: "xray_train", Intent: "train", Task: "image_classification", Records: 12000, Classes: 2, Extension: "jpg", SizeBytes: 1 << 30},
{Name: "xray_test", Intent: "test", Task: "image_classification", Records: 3000, Classes: 2, Extension: "jpg", SizeBytes: 256 << 20},
{Name: "xray_train", Intent: "train", Task: "image_classification", Records: 12000, Classes: 2, Extension: "jpg", SizeBytes: 1 << 30, SizeKnown: true},
{Name: "xray_test", Intent: "test", Task: "image_classification", Records: 3000, Classes: 2, Extension: "jpg", SizeBytes: 256 << 20, SizeKnown: true},
{Name: "ingest_run_journal", System: true},
}
dataListFile := doc(
Expand Down
21 changes: 16 additions & 5 deletions internal/cli/data_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,13 +284,19 @@ func datasetRow(d push.DatasetInfo, modality string, nameW, recW, sizeW, fmtW in
relativeTime(d.CreatedUnix)
}

// sizeCell renders a dataset's size, or an em dash when the du size is unknown
// (jobs-manager unreachable, or a system table that isn't du-sized).
// sizeCell renders a dataset's size, or an em dash when the size is genuinely
// UNKNOWN — jobs-manager unreachable, or a system table that isn't du-sized.
//
// A measured zero is not unknown. Gating on `SizeBytes > 0` conflated the two, so a
// dataset whose bytes measured 0 was indistinguishable from one the CLI never managed
// to measure. That is how a whole modality could look sizeless while nothing was
// actually wrong with the lookup: on a bind-mounted host filesystem `du` reports 0
// blocks for small files, so every text dataset rendered "—". Report what we know.
func sizeCell(d push.DatasetInfo) string {
if d.SizeBytes > 0 {
return push.HumanBytes(d.SizeBytes)
if !d.SizeKnown {
return "—"
}
return "—"
return push.HumanBytes(d.SizeBytes)
}

// dispW is a string's width in display columns (runes), not bytes — so the em
Expand Down Expand Up @@ -504,6 +510,10 @@ type datasetJSON struct {
Classes int64 `json:"classes,omitempty"`
Format string `json:"format"`
SizeBytes int64 `json:"size_bytes"`
// size_known distinguishes a MEASURED zero from "we couldn't measure it". Without it a
// consumer reading size_bytes: 0 has the same ambiguity the human view had — which is
// how an entire modality read as sizeless while the lookup was fine (#491).
SizeKnown bool `json:"size_known"`
Ingested string `json:"ingested,omitempty"`
System bool `json:"system,omitempty"`
}
Expand Down Expand Up @@ -534,6 +544,7 @@ func writeDataListJSON(w io.Writer, namespace, release string, infos []push.Data
Classes: d.Classes,
Format: formatCell(d, m),
SizeBytes: d.SizeBytes,
SizeKnown: d.SizeKnown,
Ingested: ingestedISO(d.CreatedUnix),
System: d.System,
})
Expand Down
77 changes: 68 additions & 9 deletions internal/cli/data_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,16 @@ import (
// sample datasets spanning every modality + a system table.
func sampleInfos() []push.DatasetInfo {
return []push.DatasetInfo{
{Name: "image_train", Intent: "train", Records: 20, Classes: 2, Extension: "jpg", SizeBytes: 13210,
{Name: "image_train", Intent: "train", Records: 20, Classes: 2, Extension: "jpg", SizeBytes: 13210, SizeKnown: true,
CreatedUnix: 1721556000,
Columns: []string{"id", "label", "data_intent", "data_id", "filename", "extension"}},
{Name: "text_test", Intent: "test", Records: 10, Classes: 2, Extension: "txt", SizeBytes: 770,
{Name: "text_test", Intent: "test", Records: 10, Classes: 2, Extension: "txt", SizeBytes: 770, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "filename", "extension"}},
{Name: "tabular_train", Intent: "train", Records: 20, Classes: 2, Extension: "", SizeBytes: 206,
{Name: "tabular_train", Intent: "train", Records: 20, Classes: 2, Extension: "", SizeBytes: 206, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "age", "income"}},
{Name: "timeseries_train", Intent: "train", Records: 36, Classes: 2, Extension: "", SizeBytes: 695,
{Name: "timeseries_train", Intent: "train", Records: 36, Classes: 2, Extension: "", SizeBytes: 695, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "sequence_id", "timestamp", "hr", "temp"}},
{Name: "tracebloc_ingest_runs", SizeBytes: 4096, System: true,
{Name: "tracebloc_ingest_runs", SizeBytes: 4096, SizeKnown: true, System: true,
Columns: []string{"ingestor_id", "table_name", "registered"}},
}
}
Expand Down Expand Up @@ -124,11 +124,11 @@ func TestDatasetRow_EmptyIsWarned(t *testing.T) {
func TestRenderDataList_ColumnsAlign(t *testing.T) {
infos := []push.DatasetInfo{
// small: "5 documents" / "0.75 KiB" (both short)
{Name: "text_small", Intent: "train", Records: 5, Classes: 2, Extension: "txt", SizeBytes: 770,
{Name: "text_small", Intent: "train", Records: 5, Classes: 2, Extension: "txt", SizeBytes: 770, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "filename", "extension"}},
// wide: "100000 documents" (16) + "100.00 KiB" (10) — both overflow the
// old fixed %-12s / %9s and would shift later columns without dynamic sizing.
{Name: "text_big", Intent: "test", Records: 100000, Classes: 2, Extension: "txt", SizeBytes: 102400,
{Name: "text_big", Intent: "test", Records: 100000, Classes: 2, Extension: "txt", SizeBytes: 102400, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "filename", "extension"}},
}
var buf bytes.Buffer
Expand Down Expand Up @@ -215,9 +215,9 @@ func TestGroupLabel(t *testing.T) {
// generic modality), ordered by modality family (Image before Tabular family).
func TestRenderDataList_GroupsByTask(t *testing.T) {
infos := []push.DatasetInfo{
{Name: "sepsis_train", Task: "time_series_classification", Intent: "train", Records: 4000, Classes: 2, SizeBytes: 20480,
{Name: "sepsis_train", Task: "time_series_classification", Intent: "train", Records: 4000, Classes: 2, SizeBytes: 20480, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "sequence_id", "timestamp", "hr"}},
{Name: "xray_train", Task: "image_classification", Intent: "train", Records: 50, Classes: 2, Extension: "jpg", SizeBytes: 1048576,
{Name: "xray_train", Task: "image_classification", Intent: "train", Records: 50, Classes: 2, Extension: "jpg", SizeBytes: 1048576, SizeKnown: true,
Columns: []string{"id", "label", "data_intent", "data_id", "filename", "extension"}},
}
var buf bytes.Buffer
Expand Down Expand Up @@ -357,3 +357,62 @@ func TestWriteDataListJSON(t *testing.T) {
t.Errorf("--all JSON should include the system table (count 5), got %d", got.Count)
}
}

// A measured zero and "couldn't measure" are different facts. sizeCell gated on
// `SizeBytes > 0`, which rendered both as "—" — so an entire modality looked sizeless
// while the lookup was working fine (du reports 0 blocks for small files on a
// bind-mounted host filesystem, which is every text dataset).
func TestSizeCellDistinguishesUnknownFromZero(t *testing.T) {
for _, tc := range []struct {
name string
d push.DatasetInfo
want string
}{
{"never measured renders unknown", push.DatasetInfo{SizeKnown: false}, "—"},
{"measured zero is NOT unknown", push.DatasetInfo{SizeKnown: true, SizeBytes: 0}, "0 B"},
{"measured sub-KiB reports bytes", push.DatasetInfo{SizeKnown: true, SizeBytes: 512}, "512 B"},
{"measured real size", push.DatasetInfo{SizeKnown: true, SizeBytes: 184320}, "180.00 KiB"},
} {
if got := sizeCell(tc.d); got != tc.want {
t.Errorf("%s: sizeCell() = %q, want %q", tc.name, got, tc.want)
}
}
}

// The JSON view must carry the same distinction as the human view. Without size_known, a
// consumer reading `size_bytes: 0` has exactly the ambiguity that made an entire modality
// look sizeless while the lookup was fine (#491) — fixing only the rendered table would
// have left every scripted consumer with the original bug.
func TestDataListJSONExposesSizeKnown(t *testing.T) {
var buf bytes.Buffer
writeDataListJSON(&buf, "ns", "rel", []push.DatasetInfo{
{Name: "measured_zero", Extension: "txt", Records: 5, SizeKnown: true, SizeBytes: 0},
{Name: "unmeasured", Extension: "jpg", Records: 6},
}, false)

var out struct {
Details []struct {
Name string `json:"name"`
SizeBytes int64 `json:"size_bytes"`
SizeKnown bool `json:"size_known"`
} `json:"details"`
}
if err := json.Unmarshal(buf.Bytes(), &out); err != nil {
t.Fatalf("unmarshal: %v\n%s", err, buf.String())
}
rows := out.Details
if len(rows) != 2 {
t.Fatalf("expected 2 rows, got %d", len(rows))
}
if !rows[0].SizeKnown || rows[0].SizeBytes != 0 {
t.Errorf("a measured zero must report size_known=true with 0 bytes, got known=%v bytes=%d",
rows[0].SizeKnown, rows[0].SizeBytes)
}
if rows[1].SizeKnown {
t.Errorf("an unmeasured dataset must report size_known=false")
}
// Must be present even when false/zero — omitempty here would recreate the ambiguity.
if !strings.Contains(buf.String(), `"size_known"`) {
t.Errorf("size_known must always be emitted, got %s", buf.String())
}
}
26 changes: 25 additions & 1 deletion internal/push/list_detailed.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ type DatasetInfo struct {
Classes int64 // COUNT(DISTINCT label); 0 when unlabelled
Extension string // per-row file extension (jpg/png/txt); "" for CSV tasks
SizeBytes int64 // dataset size: du of the shared PVC for file datasets, else the DB data_length; 0 if unavailable
SizeKnown bool // whether SizeBytes was actually MEASURED. 0 bytes and "couldn't measure" are different facts and must not render alike
DBBytes int64 // information_schema.data_length — the size source for row-based (non-file) datasets
CreatedUnix int64 // create_time as a UTC epoch (tz-safe) — the sole time SoT, for both "ago" and JSON
Columns []string // all column names — drives modality inference
Expand Down Expand Up @@ -79,9 +80,11 @@ func applyDatasetSizes(infos []DatasetInfo, duSizes map[string]int64) {
if infos[i].Extension != "" {
if b, ok := duSizes[infos[i].Name]; ok {
infos[i].SizeBytes = b // file dataset: real PVC size
infos[i].SizeKnown = true
}
} else if infos[i].Records > 0 {
infos[i].SizeBytes = infos[i].DBBytes // row-based with rows: DB data_length
infos[i].SizeKnown = true
}
}
}
Expand All @@ -95,17 +98,38 @@ func datasetSizesFromShared(ctx context.Context, exec Executor, cs kubernetes.In
return nil
}
var stdout, stderr bytes.Buffer
// --apparent-size measures st_size, plain `du -sk` measures st_blocks. That
// distinction is not cosmetic: on a bind-mounted host filesystem (Docker Desktop
// on Windows, and any mount that doesn't report block allocation for small files)
// st_blocks comes back 0 for a directory of small files, so every text dataset --
// dozens of small .txt documents -- measured 0 bytes and rendered as if its size
// were unknown, while image datasets measured fine because their files are large
// enough to occupy reported blocks.
//
// --apparent-size is GNU coreutils; busybox du rejects it. Probe support once on a
// trivial path and only then use it, rather than running the real du twice: du exits
// non-zero if ANY entry is unreadable, so a `du --apparent-size … || du …` chain
// would silently fall back to block sizes whenever a single path was unreadable --
// reintroducing the bug intermittently, which is worse than never having the flag.
//
// `|| true`: du exits non-zero if ANY entry is unreadable (jobs-manager can
// hit EACCES on some shared-PVC paths), but the readable entries are already
// on stdout — keep them rather than blanking every dataset's size.
if err := exec.Exec(ctx, namespace, pod, container,
[]string{"sh", "-c", "du -sk " + SharedRoot + "/* 2>/dev/null || true"},
[]string{"sh", "-c", duSharedCmd()},
nil, &stdout, &stderr); err != nil {
return nil
}
return parseDuOutput(stdout.String())
}

// duSharedCmd is the shell run inside jobs-manager to size the shared PVC. Kept as a
// function so tests assert the string that actually ships rather than a copy of it.
func duSharedCmd() string {
return "A=; du -sk --apparent-size /dev/null >/dev/null 2>&1 && A=--apparent-size; " +
"du -sk $A " + SharedRoot + "/* 2>/dev/null || true"
}

// parseDuOutput parses `du -sk` output ("<KiB>\t<path>" per line) into a
// name→bytes map keyed by the path's basename (the dataset name).
func parseDuOutput(raw string) map[string]int64 {
Expand Down
54 changes: 54 additions & 0 deletions internal/push/list_detailed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,57 @@ func TestListDatasetsDetailedWith_SkipsTaskWhenColumnAbsent(t *testing.T) {
t.Errorf("task lookup must be skipped when the column is absent; got %d queries", len(fe.queries))
}
}

// --- text datasets measured 0 bytes and rendered as "unknown" ---------------

// du -sk measures st_blocks. On a bind-mounted host filesystem (Docker Desktop on
// Windows, and any mount that doesn't report block allocation for small files) that
// comes back 0 for a directory of small files -- so every text dataset (dozens of
// small .txt documents) measured 0 bytes, while image datasets measured fine because
// their files are large enough to occupy reported blocks. --apparent-size measures
// st_size instead, which is the number a user means by "how big is my dataset".
func TestDuCommandMeasuresApparentSize(t *testing.T) {
cmd := duSharedCmd()
if !strings.Contains(cmd, "--apparent-size") {
t.Fatalf("du must measure apparent size, not disk blocks: %q", cmd)
}
// busybox du rejects the flag, so support has to be probed rather than assumed.
if !strings.Contains(cmd, "/dev/null") {
t.Errorf("expected a cheap support probe before using the flag: %q", cmd)
}
// The trap: `du --apparent-size … || du …` looks equivalent but is not. du exits
// non-zero if ANY path is unreadable, so that chain silently falls back to block
// sizes whenever a single dataset dir is inaccessible -- reintroducing this bug
// intermittently, which is harder to diagnose than never having the flag.
if strings.Count(cmd, "du -sk $A") != 1 {
t.Errorf("the real du must run exactly once, parameterised by the probe: %q", cmd)
}
}

func TestApplyDatasetSizesRecordsWhetherItMeasured(t *testing.T) {
infos := []DatasetInfo{
{Name: "txt_small", Extension: "txt", Records: 120}, // measured, but 0 bytes
{Name: "img", Extension: "jpg", Records: 6}, // measured, real bytes
{Name: "unmeasured", Extension: "jpg", Records: 6}, // no du entry at all
{Name: "rows", Extension: "", Records: 8, DBBytes: 16384}, // row-based
{Name: "empty", Extension: "", Records: 0, DBBytes: 16384}, // empty table
}
applyDatasetSizes(infos, map[string]int64{"txt_small": 0, "img": 184320})

for _, tc := range []struct {
i int
wantKnown bool
wantBytes int64
}{
{0, true, 0}, // a measured zero is KNOWN — the whole point
{1, true, 184320}, //
{2, false, 0}, // genuinely unknown: du had no entry
{3, true, 16384}, //
{4, false, 0}, // empty table: data_length is an InnoDB page, not data
} {
if infos[tc.i].SizeKnown != tc.wantKnown || infos[tc.i].SizeBytes != tc.wantBytes {
t.Errorf("%s: got known=%v bytes=%d, want known=%v bytes=%d",
infos[tc.i].Name, infos[tc.i].SizeKnown, infos[tc.i].SizeBytes, tc.wantKnown, tc.wantBytes)
}
}
}
Loading