oxidecomputer / oxidecomputer/omicron

`omdb saga abandon` could try to warn you about unlocking locked instances (and other orphaned state?)

Open
#10,263 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

As discussed in #10222:

In general, using omdb saga abandon can create state that requires manual cleanup. Has someone assessed whether some cleanup is required on dogfood here?

at minimum, the instance lock is probably still taken for that instance, and will need to be manually unlocked. maybe there's an omdb command for this, or maybe there should be? paging @hawkw

Ah, sorry, I missed this on Friday! I don't actually think that the instance-updater lock is an issue here; that is only used by instance_update sagas, and it looks like the stuck saga here is an instance_start rather than an instance_update, so we don't need to worry about leaving behind a stale instance lock when abandoning an instance start saga.

I think OMDB should probably warn you about the updater lock specifically when you're abandoning an instance-update saga, though, and it might also be a good idea to build an additional OMDB command to forcefully unlock an instance. Such a command could take the UUID of the saga which has been abandoned and could unlock the instance only if that ID is the one currently holding the lock, which is a bit safer than manually running some SQL to set the lock to NULL unconditionally.

Originally posted by @hawkw in #10222

I think we should at minimum make the omdb saga abandon command warn the user about the instance-updater lock when abandoning an instance_update saga. Ideally, we would look at the saga nodes and determine whether it had acquired a lock, and give you the instance lock UUID you could use to unlock the instance record.

It would also be cool if it knew how to go through and tell you about all the database records that any saga may have created, and told you to clean them up, but I think this would be much more difficult and flaky, since we'd need the saga abandon command to be aware of the semantics of every node in every saga. If we wanted to do that, we'd probably want some codegen-y way to annotate a node as "creating these DB records", which is going to be a bother, and also seems duplicative of the already existing unwind actions defined for that saga. But, I think that leaving an instance permanently locked for update is bad enough, and narrowly scoped enough, that it seems worth special-casing that one.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the omdb saga abandon command and trace how it identifies saga types and inspects saga nodes. For an abandoned instance_update saga, determine whether an instance-updater lock was acquired and report the lock UUID or required cleanup; done means the command warns clearly without attempting broad, unreliable orphan-state detection.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cli
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.