Skip to content

[nexus] instance update saga: retry "inherit lock", too #10801

Merged
hawkw merged 2 commits into
mainfrom
eliza/inherit-lock
Jul 17, 2026
Merged

[nexus] instance update saga: retry "inherit lock", too #10801
hawkw merged 2 commits into
mainfrom
eliza/inherit-lock

Conversation

@hawkw

@hawkw hawkw commented Jul 10, 2026

Copy link
Copy Markdown
Member

Similarly to #10166, a duplicate execution of the siu_become_updater
saga action, which attempts to inherit the lock from the parent
start_instance_update saga, must never fail if a previous execution of
that node has already marked the instance record as locked. Like in PR
#10177, which fixed #10166, we can resolve this by adding a retry loop
that should retry any transient database errors indefinitely. And, as we
discovered in #10785, the exception to this rule is cases where the
instance record has been deleted, so we handle that gracefully here as
well (or at least we will, once #10787 actually fixes the lock
inheriting query's handling of deleted instance records...).

hawkw added 2 commits July 10, 2026 14:56
Similarly to #10166, a duplicate execution of the `siu_become_updater`
saga action, which attempts to inherit the lock from the parent
`start_instance_update` saga, must never fail if a previous execution of
that node has already marked the instance record as locked. Like in PR
#10177, which fixed #10166, we can resolve this by adding a retry loop
that should retry any transient database errors indefinitely. And, as we
discovered in #10785, the exception to this rule is cases where the
instance record has been deleted, so we handle that gracefully here as
well (or at least we will, once #10787 actually fixes the lock
inheriting query's handling of deleted instance records...).
@hawkw
hawkw requested review from davepacheco and jmpesp July 10, 2026 21:59

@nickelization nickelization left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only my third day at Oxide, so there's a chance you might want to wait for someone more experienced to review this, but for what it's worth I looked it over pretty carefully and everything here makes sense to me and looks good!

Really appreciated the detailed comments by the way - as a newbie they were definitely helpful to me in understanding what's going on here, and also helped solidify my thinking around this stuff in general a bit too.

@hawkw

hawkw commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Really appreciated the detailed comments by the way - as a newbie they were definitely helpful to me in understanding what's going on here, and also helped solidify my thinking around this stuff in general a bit too.

Thanks, I'm glad you found them helpful! I try my best. :)

@hawkw
hawkw merged commit 6de0cc1 into main Jul 17, 2026
19 checks passed
@hawkw
hawkw deleted the eliza/inherit-lock branch July 17, 2026 20:49
hawkw added a commit that referenced this pull request Jul 17, 2026
PR #10801 changed one of the arguments to
`instance_updater_inherit_lock` to be passed as a reference, and #10787
added a new test that calls that function. I branched #10801 from `main`
prior to #10787 merging, so the `&` was never added where it needed to
be. Hilarity ensues. :(
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.

a well-timed sequence of a Nexus crash and transient db error can leave an instance locked by a failed update saga forever

2 participants