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

Remove more unnecessary DsState variants #1550

Merged
merged 2 commits into from
Nov 5, 2024
Merged

Conversation

mkeeter
Copy link
Contributor

@mkeeter mkeeter commented Nov 1, 2024

(staged on top of #1549)

  • DsState::Migrating is not yet implemented
  • DsState::Disconnected is another transient state (see Remove transient states in DsState #1526). It's assigned in DownstairsClient::restart_connection (which halts the IO task), then we switch to DsState::New once the IO task halts. Those two states are never actually handled differently, so we can switch to DsState::New immediately

@mkeeter mkeeter force-pushed the mkeeter/remove-more-states branch 3 times, most recently from a6d1a0d to 81b65c5 Compare November 1, 2024 20:23
@mkeeter mkeeter force-pushed the mkeeter/consolidate-reinitialize branch from 7c40c94 to a9d7090 Compare November 1, 2024 20:49
@mkeeter mkeeter force-pushed the mkeeter/remove-more-states branch 2 times, most recently from 9f06089 to 9d73e80 Compare November 1, 2024 21:07
* │ │ ▼ └───►───┐ │ ║ ║
* │ bad│ ┌────┴──────┐ │ │ ║ ║
* │ region│ │ Wait │ │ │ ║ ║
* │ │ ▼ └───►───┐ other │ ║ ║
Copy link
Contributor

Choose a reason for hiding this comment

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

Heh, we probably don't need the arrow going form new -> new.

Comment on lines +718 to +719
* │ │ ▼ └───►───┐ other │ ║ ║
* │ bad│ ┌────┴──────┐ │ failures ║ ║
Copy link
Contributor

Choose a reason for hiding this comment

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

Any other failures, other than the connection going away?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, going back to New is the universal behavior for any failures before going active.

Base automatically changed from mkeeter/consolidate-reinitialize to main November 5, 2024 20:46
@mkeeter mkeeter force-pushed the mkeeter/remove-more-states branch from 9d73e80 to f610a22 Compare November 5, 2024 20:49
@mkeeter mkeeter merged commit 6277bf3 into main Nov 5, 2024
16 checks passed
@mkeeter mkeeter deleted the mkeeter/remove-more-states branch November 5, 2024 21:36
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.

3 participants