Skip to content

fix(cache): build RuntimeInfo from the worker tiered store - #6181

Open
btxu-db wants to merge 1 commit into
fluid-cloudnative:masterfrom
btxu-db:fix/cacheruntime-tieredstore-node-labels
Open

fix(cache): build RuntimeInfo from the worker tiered store#6181
btxu-db wants to merge 1 commit into
fluid-cloudnative:masterfrom
btxu-db:fix/cacheruntime-tieredstore-node-labels

Conversation

@btxu-db

@btxu-db btxu-db commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

CacheEngine.getRuntimeInfo built its RuntimeInfo with an empty TieredStore, so convertToTieredstoreInfo returned no levels, GetLevelStorageMap returned an empty map, and labelNodeWithCapacityInfo never added the memory or disk capacity labels while writing the total one unconditionally as 0B. A CacheRuntime declaring a real tiered store advertised no cache capacity on any node it landed on.

Every other engine passes its runtime's tiered store here; CacheRuntime was the only one passing a zero value, because RuntimeTieredStore and the legacy TieredStore that base.WithTieredStore consumes describe the storage medium differently and no conversion existed.

This PR adds convertToLegacyTieredStore to bridge the two, and wires it into getRuntimeInfo.

Two points from the issue that were product decisions rather than mechanical ones:

  • Medium mapping. The legacy Level carries an explicit MEM/SSD/HDD enum while RuntimeTieredStoreLevel names the medium structurally through ProcessMemory, EmptyDir and HostPath. Only the memory-versus-disk distinction survives, which is all the consumers need — GetLevelStorageMap buckets SSD and HDD together. Disk-backed levels are reported as HDD, matching the medium type extractTieredStoreLevels already writes into the runtime config ConfigMap.
  • Whether the client tier counts. No. Only the worker tiered store feeds RuntimeInfo. Nodes are labelled off worker pod placement in getDesiredNodesWithScheduleInfo, so a client tier would be counted on nodes where a worker happens to co-reside and silently dropped everywhere else.

Host path levels are converted through QuotaList rather than Quota, so per-path quotas keep their declared distribution instead of being averaged across the paths by convertToTieredstoreInfo.

Ⅱ. Does this pull request fix one issue?

fixes #6174

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Unit tests for convertToLegacyTieredStore in pkg/ddc/cache/engine/transform_tiered_store_test.go, covering:

  • an unset RuntimeTieredStore producing no levels
  • ProcessMemory mapping to MEM with the /dev/shm path, and the high / low watermarks carried through
  • EmptyDir with medium: Memory mapping to MEM, and with the default medium to a disk medium
  • HostPath keeping its per-path quotas instead of collapsing them into one
  • a HostPath level whose paths and quotas disagree in length being skipped rather than emitted
  • a level naming no medium being skipped
  • multiple levels keeping their declared order, with paths indexed per level

Plus one test that feeds the result through convertToTieredstoreInfo to check the levels are accepted and their quotas summed correctly, which is the actual path from this conversion to the node labels.

Ⅳ. Describe how to verify it

go test ./pkg/ddc/cache/engine/... ./pkg/ddc/base/... -gcflags=all=-l

End to end, with the CacheRuntime from the issue:

spec:
  worker:
    replicas: 2
    tieredStore:
      levels:
      - emptyDir: {quota: 1Gi}
        high: "0.8"
        low: "0.5"
kubectl get node <node> -o json | jq '.metadata.labels | with_entries(select(.key | contains("fluid.io")))'

fluid.io/s-h-cache-t-default-mooncake-demo now reports 1GiB instead of 0B, and fluid.io/s-h-cache-d-default-mooncake-demo is present. Switching the level to emptyDir: {quota: 1Gi, medium: Memory} or to processMemory: {quota: 1Gi} yields fluid.io/s-h-cache-m-... instead.

Ⅴ. Special notes for reviews

The capacity labels are written once, when a node first enters the cache node set: calculateNodeDifferences only visits newly added nodes, and addScheduleInfoToNode skips a node that already carries the runtime label. A CacheRuntime that existed before this change therefore keeps its 0B label until it is recreated. Worth deciding separately whether that is worth addressing.

Note this is a separate problem from the tiered store memory quota being dropped from the worker memory limit (#6166, fixed by #6167): that one concerns the container's memory accounting, this one concerns the node labels. They share the tieredStore spec field but neither fix depends on the other.

@fluid-e2e-bot

fluid-e2e-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign trafalgarzzz for approval by writing /assign @trafalgarzzz in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fluid-e2e-bot

fluid-e2e-bot Bot commented Sep 3, 2026

Copy link
Copy Markdown

Hi @btxu-db. Thanks for your PR.

I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@btxu-db
btxu-db force-pushed the fix/cacheruntime-tieredstore-node-labels branch 2 times, most recently from e925c57 to baee002 Compare September 3, 2026 15:37
CacheEngine.getRuntimeInfo built its RuntimeInfo with an empty TieredStore,
so convertToTieredstoreInfo returned no levels, GetLevelStorageMap returned an
empty map, and labelNodeWithCapacityInfo never added the memory or disk capacity
labels while writing the total one unconditionally as 0B. A CacheRuntime
declaring a real tiered store advertised no cache capacity on any node it landed
on. Every other engine passes its runtime's tiered store here; CacheRuntime was
the only one passing a zero value, because RuntimeTieredStore and the legacy
TieredStore that base.WithTieredStore consumes describe the storage medium
differently and no conversion existed.

convertToLegacyTieredStore bridges the two. The legacy Level carries an explicit
MEM/SSD/HDD enum while RuntimeTieredStoreLevel names the medium structurally, so
only the memory-versus-disk distinction survives - which is all the consumers
need, as GetLevelStorageMap buckets SSD and HDD together. Disk-backed levels are
reported as HDD to match the medium type extractTieredStoreLevels already writes
into the runtime config ConfigMap. Host path levels are converted through
QuotaList rather than Quota so that per-path quotas keep their declared
distribution instead of being averaged across the paths.

Only the worker tiered store feeds RuntimeInfo. Nodes are labelled off worker
pod placement in getDesiredNodesWithScheduleInfo, so a client tier would be
counted where a worker happens to co-reside and dropped everywhere else.

Note that the capacity labels are written once, when a node first enters the
cache node set: calculateNodeDifferences only visits newly added nodes and
addScheduleInfoToNode skips a node that already carries the runtime label. A
CacheRuntime that existed before this change therefore keeps its 0B label until
it is recreated.

Fixes fluid-cloudnative#6174

Signed-off-by: btxu-db <btxu-db@outlook.com>
@btxu-db
btxu-db force-pushed the fix/cacheruntime-tieredstore-node-labels branch from baee002 to 68e98ac Compare September 3, 2026 15:47
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.28%. Comparing base (f2785f8) to head (68e98ac).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6181      +/-   ##
==========================================
+ Coverage   65.24%   65.28%   +0.04%     
==========================================
  Files         486      486              
  Lines       34194    34239      +45     
==========================================
+ Hits        22309    22354      +45     
  Misses      10135    10135              
  Partials     1750     1750              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]CacheRuntime: RuntimeInfo is built with an empty TieredStore, so cache capacity node labels are never populated

1 participant