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

Finality States #9

Merged
merged 5 commits into from
May 11, 2020
Merged

Finality States #9

merged 5 commits into from
May 11, 2020

Conversation

hannahhoward
Copy link
Contributor

Goals

  • cleanup go routines for state machines that are not receiving events any longer
  • provide a way to identify FSM's that have reached a state of finality -- i.e. their state will no longer change, they should reject all new events and not consume resources
  • this is relevant for the ability to restart -- we need need to be able to distinguish between deals that are in some kind of completion vs those that might need to be revisited. While this could be done manually in other code bases, it seems like a good idea to codify it as it also provides tangible benefits in terms of cleaning up go routine usage. Also, a notion of a set of finality states is part of many traditional FSM definitions.

Implementation

  • Provide a special error code which returned by a Planner signifies a normal shutdown -- i.e. not an error, but simply that the statemachine is in a state where no further transitions will occur. Also return this error when messages are sent to closed state machine.
  • Add a list of FinalityStates to FSM parameters. Check for the FSM being in a finality state inside it's handler, and shut down if neccesary
  • Make sure to clear out all FSM events when shutting down to support SendSync
  • Add an IsTerminated function of the FSM group which returns whether an FSM is in a final state
    I envision a restart operations looking like this...
var states []SomeFSMState
fsmGroup.List(&states)
for _, state := range states {
   if !fsmGroup.IsTerminated(state) {
      fsmGroup.Send(identifier(state), "restart")
   }
}
  • Add FinalityStates to docs

For discussion

In the commit history, you can see I also explored cleaning up the actual statemachine map in statemachine.Group. I ultimately found this untestable and ultimately opted for a more minimal PR that just makes sure we don't leave go routines running longer than they need to. We can however explore this in the future.

support ability to shot down a statemachine and have it treated as a non-error, also remove
statemachines from active list when shutdown
add states for a statemachine in which the state machine will shut down and stop handling incoming
requests
Remove unneccesary add ons to state machines
@hannahhoward hannahhoward requested a review from ingar March 14, 2020 01:02
Copy link
Contributor

@ingar ingar left a comment

Choose a reason for hiding this comment

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

Couple of tweaks if you like, otherwise 👍 Nice to not have deals come back alive out of errored states!

fsm/eventprocessor.go Show resolved Hide resolved
fsm/fsm.go Show resolved Hide resolved
@ingar ingar merged commit 9ee24d3 into master May 11, 2020
@ingar ingar deleted the feat/finality_states branch May 11, 2020 20:17
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