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

GetSync method #16

Merged
merged 3 commits into from Jul 14, 2020
Merged

GetSync method #16

merged 3 commits into from Jul 14, 2020

Conversation

hannahhoward
Copy link
Collaborator

Goals

Often we want to dispatch events and then read the result. However, currently, Get reads from the datastore immediately, while the events may still not have been processed in thread. GetSync allows you to get the value with a guarantee that any events you've already dispatched will be processed before your value is read.

Implementation

  • GetSync works by using the existing SendSync functionality and dispatching a hidden event on call. Essentially, send a hidden event, wait for it to complete (since events are processed in order, this means any other events will already be processed), then read the value from the datastore

Add a method that will insure our events are processed before we return a state value
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.

Nice solution using SendSync. Looks good, not sure if the concern about a race is warranted.

fsm/fsm_group.go Outdated
Comment on lines 50 to 54
err := s.SendSync(ctx, id, nilEvent{})
if err != nil {
return err
}
return s.Get(id).Get(out)
Copy link
Contributor

Choose a reason for hiding this comment

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

Was wondering if there was a chance for a race here? I'm not sure how GetSync gets called in the loop; is it possible that more events have come in after the nilEvent, between the SendSync and Get calls? Not sure that it matters for our use case.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, it is technically possible. I'm not concerned. The main concern is anything that happened before GetSync was called is processed. That's the guarantee GetSync offers.

@hannahhoward hannahhoward merged commit d365f65 into master Jul 14, 2020
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.

None yet

2 participants