issue-5588: [Tasks] support non-cancellable tasks that can only either complete successfully or fail with a NonCancellableError by SvartMetal · Pull Request #5589 · ydb-platform/nbs

SvartMetal · 2026-03-26T00:54:24Z

Notes

Needed to mitigate the impact of bugs like #5511.

If a DeleteDisk task encounters a non-retriable error, it persists that error in its task state.
As a result, the corresponding Compute API operation becomes stuck, because it keeps retrying the same Disk Manager task and gets the same error every time.
Cleanup tasks should not be allowed to fail with retriable or non-retriable errors.
The only exception is a non-cancellable error, which means the task itself must not run and therefore must not be cancelled, to avoid further damaging the system.
In other words, cleanup tasks should always finish successfully, regardless of fatal errors returned by underlying services.
Such errors should still be reflected in metrics, but they should not cause the task to fail.
Whenever a cleanup task finishes unsuccessfully, this indicates an abnormal situation that likely requires manual intervention from the on-duty engineer

Issue

#5588

cloud/tasks/runner.go

github-actions · 2026-03-26T01:18:06Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 304s): all tests PASSED for commit 91d4b72.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
878	878	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 1972s): all tests PASSED for commit 91d4b72.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
1414	1414	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7136s): all tests PASSED for commit 91d4b72.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3558	3558	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 11263s): all tests PASSED for commit 91d4b72.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
6401	6400	0	0	0	1	0

github-actions · 2026-03-27T02:20:34Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7103s): all tests PASSED for commit 1b5a223.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3558	3558	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 11225s): all tests PASSED for commit 1b5a223.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
6401	6400	0	0	0	1	0

…r complete successfully or fail with a NonCancellableError

github-actions · 2026-03-27T10:19:14Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 350s): all tests PASSED for commit daf7340.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
882	882	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 1951s): all tests PASSED for commit daf7340.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
1414	1414	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7136s): all tests PASSED for commit daf7340.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3558	3558	0	0	0	0	0

🔴 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 11296s): some tests FAILED for commit daf7340.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
6401	6399	0	1	0	1	0

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 36s): all tests PASSED for commit daf7340.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
2	2	0	0	0	0	0

cloud/tasks/scheduler.go

cloud/tasks/runner.go

github-actions · 2026-03-27T13:56:58Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 402s): all tests PASSED for commit f84230d.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
882	882	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 2052s): all tests PASSED for commit f84230d.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
1414	1414	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7138s): all tests PASSED for commit f84230d.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3558	3558	0	0	0	0	0

gy2411 · 2026-03-27T15:52:50Z

cloud/tasks/storage/storage_ydb_impl.go

 	lastState := states[0]

+	// A non-cancellable task may transition to the cancelled state, as this
+	// simply skips the actual cancellation process


IsCancellationRequested checks that status in not Cancelled -- so why not just

if lastState.NonCancellable && IsCancellationRequested(state.Status) { return ... }

Thanks, fixed

gy2411 · 2026-03-27T15:58:35Z

cloud/tasks/storage/storage_ydb_impl.go

+		// It is forbidden to start the cancellation process for a
+		// non-cancellable task
+		if IsCancellationRequested(state.Status) {
+			return TaskState{}, errors.NewNonRetriableErrorf(


Seems that will we account one and the same error twice for non-cancellable tasks.

E.g. if the task got a non-retriable error, we will account the error inself in onExecutionError https://github.com/ydb-platform/nbs/pull/5589/changes#diff-e17825d18ff2e356fdaf163881058e0cc95b82194123ea7d232049b8e80c2737R178 and also account this error here https://github.com/ydb-platform/nbs/blob/main/cloud/tasks/runner.go#L236. Is it ok?

Maybe we should return InterruptExecutionError here?

I think this is acceptable - more alerts in such situations are better.

gy2411 · 2026-03-27T16:02:33Z

cloud/tasks/execution_context.go


 	GetTaskID() string

+	IsNonCancellable() bool


Maybe write a comment here https://github.com/ydb-platform/nbs/blob/main/cloud/tasks/execution_context.go#L295 that setError returns an error for non-cancellable tasks? Is doesn't seem obvious and requires diving deep into the storage code to understand it.

Added a comment

github-actions · 2026-03-27T17:12:37Z

Note

This is an automated comment that will be appended during run.

🔴 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 362s): some tests FAILED for commit 896c5ee.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
882	881	0	1	0	0	0

🔴 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 9s): some tests FAILED for commit 896c5ee.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
2	1	0	1	0	0	0

🔴 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 9s): some tests FAILED for commit 896c5ee.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
2	1	0	1	0	0	0

jkuradobery

LGTM

github-actions · 2026-03-27T17:44:03Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo target: cloud/tasks/,cloud/storage/ (test time: 468s): all tests PASSED for commit bbd126a.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
882	882	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/disk_manager/ (test time: 2111s): all tests PASSED for commit bbd126a.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
1414	1414	0	0	0	0	0

🟢 linux-x86_64-relwithdebinfo target: cloud/filestore/ (test time: 7159s): all tests PASSED for commit bbd126a.

TESTS	PASSED	ERRORS	FAILED	FAILED BUILD	SKIPPED	MUTED^?
3558	3558	0	0	0	0	0

SvartMetal added the large-tests Launch large tests for PR label Mar 26, 2026

SvartMetal commented Mar 26, 2026

View reviewed changes

cloud/tasks/runner.go Show resolved Hide resolved

SvartMetal force-pushed the users/svartmetal/issue-5588/1 branch from c1b8e71 to 91d4b72 Compare March 26, 2026 01:01

SvartMetal force-pushed the users/svartmetal/issue-5588/1 branch from 91d4b72 to cd08321 Compare March 26, 2026 23:12

SvartMetal force-pushed the users/svartmetal/issue-5588/1 branch from cd08321 to 1b5a223 Compare March 27, 2026 00:06

issue-5588: [Tasks] support non-cancellable tasks that can only eithe…

daf7340

…r complete successfully or fail with a NonCancellableError

SvartMetal force-pushed the users/svartmetal/issue-5588/1 branch from 1b5a223 to daf7340 Compare March 27, 2026 10:03

jkuradobery self-requested a review March 27, 2026 12:02

SvartMetal requested a review from gy2411 March 27, 2026 12:08

jkuradobery reviewed Mar 27, 2026

View reviewed changes

cloud/tasks/scheduler.go Outdated Show resolved Hide resolved

jkuradobery reviewed Mar 27, 2026

View reviewed changes

cloud/tasks/runner.go Show resolved Hide resolved

SvartMetal added 2 commits March 27, 2026 14:25

fix review issues

d160ba0

add tests for task panicking in Cancel

f84230d

jkuradobery previously approved these changes Mar 27, 2026

View reviewed changes

gy2411 reviewed Mar 27, 2026

View reviewed changes

SvartMetal dismissed jkuradobery’s stale review via 896c5ee March 27, 2026 16:49

gy2411 previously approved these changes Mar 27, 2026

View reviewed changes

fix review issues

bbd126a

SvartMetal dismissed gy2411’s stale review via bbd126a March 27, 2026 17:18

SvartMetal force-pushed the users/svartmetal/issue-5588/1 branch from 896c5ee to bbd126a Compare March 27, 2026 17:18

gy2411 approved these changes Mar 27, 2026

View reviewed changes

jkuradobery approved these changes Mar 27, 2026

View reviewed changes

Conversation

SvartMetal commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes

Issue

Uh oh!

Uh oh!

github-actions bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gy2411 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

SvartMetal Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

gy2411 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

SvartMetal Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

gy2411 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

SvartMetal Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkuradobery left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SvartMetal commented Mar 26, 2026 •

edited

Loading

github-actions bot commented Mar 26, 2026 •

edited

Loading

github-actions bot commented Mar 27, 2026 •

edited

Loading

github-actions bot commented Mar 27, 2026 •

edited

Loading

github-actions bot commented Mar 27, 2026 •

edited

Loading

github-actions bot commented Mar 27, 2026 •

edited

Loading

github-actions bot commented Mar 27, 2026 •

edited

Loading