Skip to content

[fix][fs] Fix warmup query hanging for 10 minutes after warmup has finished - #198

Merged
jackblack369 merged 1 commit into
mainfrom
fix/warmup-query-exit-condition
Aug 14, 2026
Merged

jackblack369 merged 1 commit into
mainfrom
fix/warmup-query-exit-condition

Conversation

@Wine93

@Wine93 Wine93 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Problem

dingo fs warmup add <path> (without --daemon) leaves the progress bar stuck at 100% for exactly 10 minutes before the command returns.

Observed on a live cluster: the warmup itself finished in ~20ms, but the CLI kept polling for 10 more minutes.

16:08:16  dingo fs warmup add ... starts
16:08:18  client: Set warmup task context: [key: 20000000001, ...]
16:08:18  all 5 blocks fetched, task complete
16:08:19  client: Get warmup task status value: 5/5/0   <- already done at the first poll
   ...    4938 polls, every 200ms, value never changes
16:18:16  process finally exits                          <- exactly 10 minutes later

Root cause

The two sides disagree on what "finished" means.

ClientWarmupManager::FinishTask keeps a completed task's snapshot in finished_status_ for kFinishedStatusTtl = 10 minutes, so GetWarmupTaskStatus keeps returning 5/5/0 for that whole window and only returns 0/0/0 once the TTL expires.

CLI — the poll loop in runQuery treated total == 0 as the only completion signal, so it had to wait out the client's TTL.

This is a regression from dingofs 8e728fdd7 ("[feat][client] Make warmup scheduling nonblocking"). Before that commit a finished task was dropped from warmup_tasks_ immediately and the status went to 0/0/0 right away, which is what the total == 0 check implicitly relied on. Retaining the finished snapshot is intentional on the client side (otherwise warmup query cannot distinguish "not started" from "just finished"), so the fix belongs here.

Fix

Break once every block is accounted for — finished + errors >= total — which is the actual completion signal. The total == 0 case is kept for when the client has already dropped the status. bar.Set64 runs before the new break so the bar ends at 100% rather than at a partial value.

Also corrects a comment and an error message that described the payload as [finished/total/errors]; the client emits [total/finished/errors] and the parsing already used that order.

Testing

  • go build and go vet pass for cli/command/fs/warmup/; gofmt clean.
  • Not verified end-to-end. go build ./... fails in this repo, but identically on a clean main (58 undefined: errors in proto/dingofs/proto/{cache,web} and internal/rpc/mds.go — the generated proto code is out of sync with the Go code). I could not produce a dingo binary to re-run the scenario against a live mount. Please build in a working environment and confirm dingo fs warmup add now returns in seconds instead of 10 minutes before merging.

The query loop only stopped when the client reported total == 0. The
client keeps a finished task's status readable for kFinishedStatusTtl
(10 minutes) and reports the completed snapshot for that whole window,
so total only drops to 0 long after the warmup itself is done. This
stalled the progress bar at 100% for a full 10 minutes on every
`dingo fs warmup add` without --daemon.

Break once every block is accounted for (finished + errors >= total),
which is the real completion signal, and keep the total == 0 case for
when the client has already dropped the status. Update the progress bar
before breaking so it ends at 100%.

Also correct the result format in a comment and an error message: the
client emits [total/finished/errors], not [finished/total/errors] (the
parsing was already using the correct order).
@jackblack369
jackblack369 added this pull request to the merge queue Aug 14, 2026
Merged via the queue into main with commit d402ebd Aug 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants