fix(cache): restore Dataset to Bound after CacheRuntime recovers from an outage - #6162
fix(cache): restore Dataset to Bound after CacheRuntime recovers from an outage#6162pujitha24 wants to merge 1 commit into
Conversation
… an outage Motivation: A CacheRuntime-backed Dataset that flips to Failed during a transient runtime outage (e.g. a worker pod restart) never returns to Bound, even after the runtime becomes fully Ready again. Only deleting and recreating the CacheRuntime restored the correct state. Approach: CacheEngine.Sync only refreshed cache states when the runtime was ready, without touching the Dataset's phase. The Bound phase was otherwise only ever set once, by BindToDataset during initial Setup, which does not run again on later reconciles, so the Failed phase was a one-way trap. Sync now checks the Dataset's current phase whenever the runtime is ready and, if it is Failed, restores it to Bound via the existing UpdateDatasetStatus helper before falling back to the regular cache-states sync. Validation: - go build ./... - go vet ./pkg/ddc/cache/... - gofmt -l pkg/ddc/cache/engine/sync.go pkg/ddc/cache/engine/sync_test.go (no output) - golangci-lint run ./pkg/ddc/cache/... -> 0 issues - go test ./pkg/ddc/cache/engine/... -run TestCacheEngine --ginkgo.focus="left Failed by a previous outage" -v -> PASS - Confirmed the new test is a genuine regression test: reverting only sync.go while keeping the new test makes it fail with Failed != Bound - Full unfocused suite in this package shows pre-existing, order-dependent flaky failures in ufs_test.go/dataset_test.go/fileutils_test.go that reproduce identically on unmodified master, unrelated to this change Report: fluid-cloudnative#6160 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @pujitha24. 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 Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6162 +/- ##
==========================================
+ Coverage 65.13% 65.19% +0.06%
==========================================
Files 485 485
Lines 34039 34053 +14
==========================================
+ Hits 22171 22202 +31
+ Misses 10127 10108 -19
- Partials 1741 1743 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Ⅰ. Describe what this PR does
Fixes a bug where a CacheRuntime-backed Dataset gets permanently stuck in
Failedphase after a transient runtime outage (e.g. a worker pod restart), even after the
runtime becomes
Readyagain.Root cause: in
pkg/ddc/cache/engine/sync.go,CacheEngine.Syncsets the Dataset toFailedwhenever the runtime isn't ready, but when the runtime becomes ready again itonly calls
syncDatasetCacheStates, which never touches.Status.Phase. TheBoundphase is otherwise only set once, by
BindToDatasetduring the initialSetup(),which does not run again on subsequent reconciles. So once a Dataset flips to
Failed,nothing ever flips it back to
Bound, even though the underlying runtime has fullyrecovered.
The fix checks, on every reconcile where the runtime is ready, whether the Dataset is
currently
Failed; if so it restores it toBoundvia the existingUpdateDatasetStatushelper (the same helperBindToDatasetuses), instead of onlyrefreshing cache states.
Ⅱ. Does this pull request fix one issue?
fixes #6160
Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Added a Ginkgo test in
pkg/ddc/cache/engine/sync_test.go:"when runtime is ready but dataset was left Failed by a previous outage" — it seeds a
Dataset with
Status.Phase = FailedDatasetPhase, makes the master/workerStatefulSets report Ready, runs
engine.Sync(ctx), and asserts the Dataset's phase isrestored to
Bound.I confirmed this test is a genuine regression test for the bug: with only the test
added and the
sync.gofix reverted, it fails withFailed != Bound; with the fixapplied, it passes.
Ⅳ. Describe how to verify it
Commands run locally (all passed):
go build ./...go vet ./pkg/ddc/cache/...gofmt -l pkg/ddc/cache/engine/sync.go pkg/ddc/cache/engine/sync_test.go(no output)golangci-lint run ./pkg/ddc/cache/...→ "0 issues"go test ./pkg/ddc/cache/engine/... -run TestCacheEngine --ginkgo.focus="left Failed by a previous outage" -v→ PASSNote: running the full
pkg/ddc/cache/enginesuite unfocused shows some pre-existing,order-dependent flaky failures in
ufs_test.go/dataset_test.go/fileutils_test.go(unrelated files). These reproduce identically on unmodified
masterwith the samerandom spec ordering (verified by stashing this change and rerunning), so they are not
caused by this change.
I did not reproduce this against a live cluster; the fix and its regression test are
scoped to a single, deterministic phase-transition defect in
Sync, which thefocused unit test above demonstrates directly.
Ⅴ. Special notes for reviews
None.