Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Terminating into (soft) Terminating and HardTerminating #791

Merged
merged 5 commits into from
Jul 24, 2024

Conversation

paulgb
Copy link
Member

@paulgb paulgb commented Jul 22, 2024

Currently, we use Terminating to represent both soft and hard terminating. If a backend is hard-terminated while it is being soft-terminated (which is allowed), a backend goes from a Terminating state to another Terminating state. This is the only allowed state transition that does not move forward in the state sequence.

The change in this PR is to split soft- and hard- Terminating states into their own separate states. Soft-termination keeps the name Terminating, and hard termination gets HardTerminating. The field termination in Terminating remains, but is deprecated, and can only be set to Soft in the constructor.

Copy link

vercel bot commented Jul 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
plane ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 23, 2024 4:02pm

@rolyatmax
Copy link
Member

I think we need to update this line, too: https://github.com/jamsocket/plane/blob/main/plane/src/controller/proxy.rs#L128

@rolyatmax
Copy link
Member

@rolyatmax
Copy link
Member

@rolyatmax
Copy link
Member

rolyatmax commented Jul 23, 2024

One thought: there will be a small window of time during a deploy where it'll be possible for p2 to receive unparseable BackendState update messages. That's only for messages that have a HardTerminating BackendStatus, though, which I'm guessing are rare. If we really wanted to avoid this, we could put the type changes in their own commit that comes first in the PR, then update plane version to that specific commit, deploy, then update to the latest commit and deploy again. It's kind of a hassle though, and might not really be worth it.

@paulgb
Copy link
Member Author

paulgb commented Jul 23, 2024

One thought: there will be a small window of time during a deploy where it'll be possible for p2 to receive unparseable BackendState update messages. That's only for messages that have a HardTerminating BackendStatus, though, which I'm guessing are rare. If we really wanted to avoid this, we could put the type changes in their own commit that comes first in the PR, then update plane version to that specific commit, deploy, then update to the latest commit and deploy again. It's kind of a hassle though, and might not really be worth it.

Terminating statuses are only created on the drone, so as long as we deploy the controller before the drones we should be fine.

@rolyatmax
Copy link
Member

@paulgb Ah great point!

@paulgb paulgb force-pushed the paul/dis-2315-implement-hardterminating-status-in-plane branch from 6dc995a to 2acaeba Compare July 23, 2024 16:01
@paulgb paulgb requested a review from rolyatmax July 23, 2024 16:02
@paulgb paulgb merged commit d9d16d9 into main Jul 24, 2024
6 checks passed
@paulgb paulgb deleted the paul/dis-2315-implement-hardterminating-status-in-plane branch July 24, 2024 14:34
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.

2 participants