Skip to content

ClusterSingletonManager lease release fault not handled on leaving#8065

Open
MattKotsenas wants to merge 1 commit intoakkadotnet:devfrom
MattKotsenas:bugfix/singleton-lease-release-fault
Open

ClusterSingletonManager lease release fault not handled on leaving#8065
MattKotsenas wants to merge 1 commit intoakkadotnet:devfrom
MattKotsenas:bugfix/singleton-lease-release-fault

Conversation

@MattKotsenas
Copy link
Contributor

Changes

When transitioning from Oldest state, the lease release in GoTo().Using() used ContinueWith(r => new ReleaseLeaseResult(r.Result)) which accesses r.Result unconditionally. If the release task faults, r.Result throws AggregateException.

I had to consult Copilot about the user impact of this issue. It says:

PipeTo delivers as Status.Failure which is an unhandled message that gets silently dropped. The operator gets no error log about the failed lease release.

However, I'm still new at Cluster, so I'm not sure if that's correct, though the task handling does appear to be a bug. As a result, I aligned with the existing fault-handling pattern in OnTransition(from, to) by checking IsCanceled/IsFaulted first and producing a ReleaseLeaseFailure message, which the FSM already handles with a proper error log.

Also add SetNextReleaseResult to TestLease (mirroring the existing SetNextAcquireResult) to enable testing lease release failures.

Checklist

…ng Oldest

When transitioning from Oldest state, the lease release in
GoTo().Using() used ContinueWith(r => new ReleaseLeaseResult(r.Result))
which accesses r.Result unconditionally. If the release task faults,
r.Result throws AggregateException, which PipeTo delivers as
Status.Failure - an unhandled message that gets silently dropped.
The operator gets no error log about the failed lease release.

Fix: align with the existing fault-handling pattern in
OnTransition(from, to) by checking IsCanceled/IsFaulted first and
producing a ReleaseLeaseFailure message, which the FSM already handles
with a proper error log.

Also add SetNextReleaseResult to TestLease (mirroring the existing
SetNextAcquireResult) to enable testing lease release failures.
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.

1 participant